misc: Add python language in RoutineCharacteristics#26962
Merged
feilong-liu merged 1 commit intoprestodb:masterfrom Jan 14, 2026
Merged
misc: Add python language in RoutineCharacteristics#26962feilong-liu merged 1 commit intoprestodb:masterfrom
feilong-liu merged 1 commit intoprestodb:masterfrom
Conversation
Contributor
Reviewer's guide (collapsed on small PRs)Reviewer's GuideAdds Python as a supported routine language and removes an overly strict constraint that limited JSON-file-based function registration to C++ UDFs only. Class diagram for updated routine language support and JSON UDF namespace managerclassDiagram
class RoutineCharacteristics {
}
class Language {
+Language SQL
+Language JAVA
+Language CPP
+Language PYTHON
-String language
+Language(String language)
+String toString()
}
RoutineCharacteristics *-- Language : contains
class JsonFileBasedFunctionNamespaceManager {
-String catalogName
+String getCatalogName()
-void populateNameSpaceManager(UdfFunctionSignatureMap udfFunctionSignatureMap)
-SqlInvokedFunction createSqlInvokedFunction(String functionName, JsonBasedUdfFunctionMetadata jsonBasedUdfFunctionMetaData)
}
class SqlInvokedFunction {
}
class UdfFunctionSignatureMap {
}
class JsonBasedUdfFunctionMetadata {
+RoutineCharacteristics getRoutineCharacteristics()
+String getSchema()
+List~String~ getParamNames()
+List~TypeSignature~ getParamTypes()
}
class RoutineCharacteristicsLanguage {
+Language getLanguage()
}
JsonFileBasedFunctionNamespaceManager ..> SqlInvokedFunction : creates
JsonFileBasedFunctionNamespaceManager ..> JsonBasedUdfFunctionMetadata : uses
JsonBasedUdfFunctionMetadata ..> RoutineCharacteristicsLanguage : uses
RoutineCharacteristicsLanguage ..> Language : returns
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Contributor
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The new
PYTHONlanguage constant uses all caps whileJAVAis mixed-case ("Java"); consider aligning the string value with the existing convention or documenting the intended casing since this may affect serialization or equality checks. - With the removal of the
checkStateinJsonFileBasedFunctionNamespaceManager, it might be worth adding a more general validation/logging path to handle unsupported or unexpectedRoutineCharacteristics.Languagevalues during function registration rather than silently accepting them.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new `PYTHON` language constant uses all caps while `JAVA` is mixed-case (`"Java"`); consider aligning the string value with the existing convention or documenting the intended casing since this may affect serialization or equality checks.
- With the removal of the `checkState` in `JsonFileBasedFunctionNamespaceManager`, it might be worth adding a more general validation/logging path to handle unsupported or unexpected `RoutineCharacteristics.Language` values during function registration rather than silently accepting them.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
dc911d7 to
faacf56
Compare
kaikalur
approved these changes
Jan 14, 2026
NikhilCollooru
approved these changes
Jan 14, 2026
This was referenced Mar 31, 2026
15 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Language in RoutineCharacteristics represents the language used in the implementation of the functions. Currently it has SQL, JAVA and CPP. I am adding one more option, python, in this PR.
In addition, I also remove the constraint of only support CPP in the
JsonFileBasedFunctionNamespaceManager.java. This check was added by me and it's because theJsonFileBasedFunctionNamespaceManagerwas intended to be used for registering CPP functions at that time. However, it's only a frontend for function registration, and has nothing to do the implementation of the functions to be registered, we do not need to limit us regarding the usage of it.Motivation and Context
As in description.
Impact
As in description.
Test Plan
Easy change
Contributor checklist
Release Notes
Please follow release notes guidelines and fill in the release notes below.