[PECOBLR-1746] Implementing support for listing procedures#1238
Open
msrathore-db wants to merge 9 commits intodatabricks:mainfrom
Open
[PECOBLR-1746] Implementing support for listing procedures#1238msrathore-db wants to merge 9 commits intodatabricks:mainfrom
msrathore-db wants to merge 9 commits intodatabricks:mainfrom
Conversation
…SQL queries Implement JDBC-compliant getProcedures and getProcedureColumns by querying information_schema.ROUTINES and information_schema.parameters via SQL. This approach works for both thrift and SEA transports without using thrift RPC. - getProcedures: queries ROUTINES filtered by routine_type='PROCEDURE' - getProcedureColumns: queries parameters joined with ROUTINES for procedures - Null catalog uses system.information_schema; specific catalog uses <catalog>.information_schema - Shared SQL builders in CommandConstants to avoid duplication across SDK/Thrift clients - Maps parameter_mode (IN/OUT/INOUT) to JDBC COLUMN_TYPE constants - Maps Databricks type names to java.sql.Types via existing getCode() Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…line helpers - Extract table names, column lists, and filter into named constants in CommandConstants - Remove getProcedureColumnPrecision/getProcedureColumnLength helpers with unused dataType param - Inline precision/length logic directly in getRowsForProcedureColumns Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ocedureColumns Unit tests (DatabricksMetadataSdkClientTest): - testListProcedures: parameterized tests for SQL generation with various catalog/schema/name combinations including null catalog (system prefix) - testListProcedureColumns: parameterized tests for SQL generation with JOIN to routines table, all filter combinations Integration test (MetadataIntegrationTests): - testGetProceduresAndProcedureColumns: creates a test procedure with IN and OUT params, verifies getProcedures returns correct metadata (name, remarks, type), verifies getProcedureColumns returns correct parameter info (column types IN=1/OUT=4, data types, ordinal positions), tests column name filtering, cleans up procedure after test Also renames ROUTINES -> routines in CommandConstants for consistency. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…I changes Add GET_PROCEDURES and GET_PROCEDURE_COLUMNS to MetadataOperationType enum. Update getResultSet/executeStatement calls to pass the new required MetadataOperationType parameter after upstream API signature change. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
String.format treats the single quote before %s as a flag character, causing UnknownFormatConversion when patterns like '%' are passed. Replace String.format with StringBuilder.append for SQL LIKE clauses. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
… '%'
The LOGGER.error(e, "message {}", e) call uses String.format internally.
When the exception message contains SQL with LIKE '%', the '%' is
interpreted as a format specifier causing UnknownFormatConversionException.
Use plain log message without exception interpolation.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
msrathore-db
force-pushed
the
PECOBLR-1746
branch
from
33cc3e9 to
32e1fcd
Compare
…m values Add GET_PROCEDURES and GET_PROCEDURE_COLUMNS to the enum count assertion and parameterized header value tests. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
gopalldb
reviewed
Mar 11, 2026
| sql.append(" AND routine_schema LIKE '").append(schemaPattern).append("'"); | ||
| } | ||
| if (procedureNamePattern != null) { | ||
| sql.append(" AND routine_name LIKE '").append(procedureNamePattern).append("'"); |
Collaborator
There was a problem hiding this comment.
Is it possible to use parameterized statements here and provide user provided values as parameters? This will prevent SQL injection issues
gopalldb
reviewed
Mar 11, 2026
| } | ||
|
|
||
| private static String getCatalogPrefix(String catalog) { | ||
| return (catalog == null) ? "system" : "`" + catalog + "`"; |
Collaborator
There was a problem hiding this comment.
what if catalogName already contains a backtick character? Should we escape that?
gopalldb
reviewed
Mar 11, 2026
| return (short) procedureColumnUnknown; | ||
| } | ||
| switch (parameterMode.toUpperCase()) { | ||
| case "IN": |
gopalldb
reviewed
Mar 11, 2026
| try { | ||
| Object val = resultSet.getObject(columnName); | ||
| return val != null ? val.toString() : null; | ||
| } catch (SQLException e) { |
gopalldb
reviewed
Mar 11, 2026
| if (val == null) return null; | ||
| if (val instanceof Number) return ((Number) val).intValue(); | ||
| return Integer.parseInt(val.toString()); | ||
| } catch (SQLException | NumberFormatException e) { |
gopalldb
reviewed
Mar 11, 2026
| if (val == null) return null; | ||
| if (val instanceof Number) return ((Number) val).shortValue(); | ||
| return Short.parseShort(val.toString()); | ||
| } catch (SQLException | NumberFormatException e) { |
gopalldb
reviewed
Mar 11, 2026
| Integer charMaxLength = getIntOrNull(resultSet, "character_maximum_length"); | ||
| Integer charOctetLength = getIntOrNull(resultSet, "character_octet_length"); | ||
| row.add(numericPrecision != null ? numericPrecision : charMaxLength); // PRECISION | ||
| row.add(charOctetLength != null ? charOctetLength : numericPrecision); // LENGTH |
Collaborator
There was a problem hiding this comment.
but numericPrecision can be null also
gopalldb
reviewed
Mar 11, 2026
| Integer numericPrecision = getIntOrNull(resultSet, "numeric_precision"); | ||
| Integer charMaxLength = getIntOrNull(resultSet, "character_maximum_length"); | ||
| Integer charOctetLength = getIntOrNull(resultSet, "character_octet_length"); | ||
| row.add(numericPrecision != null ? numericPrecision : charMaxLength); // PRECISION |
Collaborator
There was a problem hiding this comment.
charMaxLength itself can be null
gopalldb
reviewed
Mar 11, 2026
| Integer charMaxLength = getIntOrNull(resultSet, "character_maximum_length"); | ||
| Integer charOctetLength = getIntOrNull(resultSet, "character_octet_length"); | ||
| row.add(numericPrecision != null ? numericPrecision : charMaxLength); // PRECISION | ||
| row.add(charOctetLength != null ? charOctetLength : numericPrecision); // LENGTH |
Collaborator
There was a problem hiding this comment.
why is length being fallback to precision?
gopalldb
reviewed
Mar 11, 2026
| Arguments.of( | ||
| "SELECT p.specific_catalog, p.specific_schema, p.specific_name," | ||
| + " p.parameter_name, p.parameter_mode, p.is_result," | ||
| + " p.data_type, p.full_data_type," |
Collaborator
There was a problem hiding this comment.
where is full_data_type used?
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
2 participants
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
Implements
getProceduresandgetProcedureColumnsinDatabricksDatabaseMetaDataby queryinginformation_schema.routinesandinformation_schema.parametersvia SQL.Unlike other metadata operations that use
SHOWcommands or Thrift RPCs, these use direct SQL SELECT queries againstinformation_schemaviews. This works for both Thrift and SEA transports.getProcedures:
information_schema.routinesfiltered byroutine_type = 'PROCEDURE'procedureNoResult(1)getProcedureColumns:
information_schema.parametersJOINed withinformation_schema.routinesto filter for proceduresparameter_mode(IN/OUT/INOUT) to JDBC COLUMN_TYPE constants (1/4/2)java.sql.Typescodes via existinggetCode()Catalog resolution:
system.information_schema.routines(cross-catalog)<catalog>.information_schema.routinesShared SQL builders in
CommandConstantseliminate duplication between SDK and Thrift clients.Testing
Unit tests (
DatabricksMetadataSdkClientTest):listProceduresSQL generation (catalog+schema+name, null schema, null name, null catalog)listProcedureColumnsSQL generation (all filters, partial filters, all nulls)Integration test (
MetadataIntegrationTests#testGetProceduresAndProcedureColumns):jdbc_test_compute_area(x DOUBLE, y DOUBLE, OUT area DOUBLE)with COMMENTgetProceduresreturns correct name, schema, catalog, remarks, typegetProcedureColumnsreturns 3 params with correct COLUMN_TYPE (IN=1, OUT=4), DATA_TYPE (DOUBLE=8), ordinal positionsExisting tests: All 248
DatabricksDatabaseMetaDataTest+ 51DatabricksMetadataSdkClientTestpass.Additional Notes to the Reviewer
information_schema.parametersis used (notroutine_columns) becauseroutine_columnscontains table-valued function output columns, not procedure parameters.procedureNullableUnknown(2) since the server does not track parameter nullability.system.information_schemais queried which requires system table access permissions. If the user lacks this permission, the driver returns an empty result set. This will be addressed server-side in a future release.