Skip to content

[CALCITE-2317] Support JDBC DatabaseMetaData.getFunctions#2118

Closed
MalteBellmann wants to merge 1 commit into
apache:masterfrom
caespes:CALCITE/2317
Closed

[CALCITE-2317] Support JDBC DatabaseMetaData.getFunctions#2118
MalteBellmann wants to merge 1 commit into
apache:masterfrom
caespes:CALCITE/2317

Conversation

@MalteBellmann
Copy link
Copy Markdown
Contributor

DatabaseMetaData.getFunctions() on a connection to Calcite now returns UDFs.


Enumerable<MetaFunction> functions(final MetaSchema schema, final String catalog,
final Predicate1<String> functionNameMatcher) {
return functions(schema, catalog)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible to add some tests?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for reviewing, i've added a test and sorting according to the JDBC specification.

FYI, there is also a discussion in JIRA on how to handle built-in functions.

@MalteBellmann
Copy link
Copy Markdown
Contributor Author

@chunweilei do you have time to review this updated PR? I think it is ready to merge

Enumerable<MetaFunction> functions(final MetaSchema schema_, final String catalog) {
final CalciteMetaSchema schema = (CalciteMetaSchema) schema_;
Enumerable<MetaFunction> opTableFunctions = Linq4j.emptyEnumerable();
if (schema.getName().equals("metadata")) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Out of curiosity: why hardcode metadata as schema name?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The idea was: It is hardcoded in https://github.com/apache/calcite/blob/master/core/src/main/java/org/apache/calcite/jdbc/CalciteSchema.java#L525-L527 and I didn't see a better way.

But I just realized, I can use MetadataSchema.INSTANCE for the check. I will update the PR accordingly

@amaliujia
Copy link
Copy Markdown
Contributor

Overall makes sense to me

DatabaseMetaData.getFunctions() on a connection to Calcite now returns
UDFs in the respective schema and system functions in the "metadata" schema.
@amaliujia amaliujia added the LGTM-will-merge-soon Overall PR looks OK. Only minor things left. label Mar 8, 2021
+ " ]\n"
+ "}";
CalciteAssert.model(model)
.withDefaultSchema("adhoc")
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@amaliujia
Copy link
Copy Markdown
Contributor

Merged via e447ff8

@amaliujia amaliujia closed this Mar 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

LGTM-will-merge-soon Overall PR looks OK. Only minor things left.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants