Skip to content

GH-1134: add COLUMN_DEF support to JDBC getColumnns()#1139

Open
Verest wants to merge 1 commit intoapache:mainfrom
Verest:GH-1134
Open

GH-1134: add COLUMN_DEF support to JDBC getColumnns()#1139
Verest wants to merge 1 commit intoapache:mainfrom
Verest:GH-1134

Conversation

@Verest
Copy link
Copy Markdown

@Verest Verest commented May 6, 2026

What's Changed

Updates the FlightSqlColumnMetadata to contain the JDBC COLUMN_DEF field alongside getters/setters. Following exiting naming format.

Updates ArrowDatabaseMetadata to write to the pre-existing COLUMN_DEF vector as part of the getColumns() code paths.

Updates test(s) in ArrowDatabaseMetadataTest to test default value behavior.

Closes #1134.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 6, 2026

Thank you for opening a pull request!

Please label the PR with one or more of:

  • bug-fix
  • chore
  • dependencies
  • documentation
  • enhancement

Also, add the 'breaking-change' label if appropriate.

See CONTRIBUTING.md for details.

@Verest
Copy link
Copy Markdown
Author

Verest commented May 6, 2026

I am murky on the specific format for COLUMN_DEF, or where that responsibility lies. The Oracle docs mention wrapping the underling value in single quotes if it is a string, and a few other things. I suspect this format is not followed everywhere though 🤔.

Since this is a generic JDBC driver, I am cautious to make assumptions about the incoming format of the COLUMN_DEF from the server. For now, I am tentatively expecting the server-side to handle the intended parsing of the COLUMN_DEF, leaving the arrow driver to read given strings as is.

Open to suggestions here.

Aside: this PR should be an enhancement, but I do not have permission to edit the labels.

@Verest Verest marked this pull request as ready for review May 6, 2026 19:48
@xborder
Copy link
Copy Markdown
Contributor

xborder commented May 6, 2026

Should this be discussed and voted like it was done for is_update? Maybe it is overkill but I think at least apache/arrow/FlightSql.proto should be in sync.

There are other places in FlightSql.proto that also mention metadata returned that probably would need to be updated

@xborder
Copy link
Copy Markdown
Contributor

xborder commented May 6, 2026

Have you thought if it would make sense in CommandGetXdbcTypeInfo?

@ennuite
Copy link
Copy Markdown
Contributor

ennuite commented May 7, 2026

Should this be discussed and voted like it was done for is_update? Maybe it is overkill but I think at least apache/arrow/FlightSql.proto should be in sync.

There are other places in FlightSql.proto that also mention metadata returned that probably would need to be updated

Yes, the proto files in this repo should mirror the main one.

@jbonofre jbonofre added this to the 20.0.0 milestone May 7, 2026
* - ARROW:FLIGHT:SQL:IS_READ_ONLY - "1" indicates if the column is read only, "0" otherwise.
* - ARROW:FLIGHT:SQL:IS_SEARCHABLE - "1" indicates if the column is searchable via WHERE clause, "0" otherwise.
* - ARROW:FLIGHT:SQL:REMARKS - A comment describing the column.
* - ARROW:FLIGHT:SQL:COLUMN_DEF - The default value for the column.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks to have update the javadoc here.

I think it's worth to document COLUMN_DEF in the javadoc of CommandStatementQuery, CommandStatementSubstraitPlan, CommandPreparedStatementQuery. As we have REMARKS in these messages, we should have COLUMN_DEF.

@jbonofre jbonofre added the enhancement PRs that add or improve features. label May 7, 2026
@jbonofre
Copy link
Copy Markdown
Member

jbonofre commented May 7, 2026

By the way, as this PR touches .proto, worth to at discuss on the mailing list.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement PRs that add or improve features.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Consume COLUMN_DEF in JDBC Flight Driver

4 participants