GH-39558: [Java] Add SQL_ALL_TABLES_ARE_SELECTABLE, SQL_NULL_ORDERING and SQL_MAX_COLUMNS_IN_TABLE support to SqlInfoBuilder#39561
Conversation
|
|
java/flight/flight-sql/src/main/java/org/apache/arrow/flight/sql/SqlInfoBuilder.java
Show resolved
Hide resolved
…RING, SQL_MAX_COLUMNS_IN_TABLE support.
|
|
|
BTW, it seems that not all methods in |
thermo911
left a comment
There was a problem hiding this comment.
The changes are pretty simple.
There was a problem hiding this comment.
Could you add, or file an issue for, generic methods to handle arbitrary metadata entries?
There was a problem hiding this comment.
Could you give an example, please? Currently, SqlInfoBuilder has private methods like withEnumProvider, withBooleanProvider etc. used by public methods like withSqlAllTablesAreSelectable. Should API be extended with smth similar?
There was a problem hiding this comment.
Right, I think we could basically make those public or something similar, to give future users an escape hatch.
There was a problem hiding this comment.
(Of course, I appreciate that you took the time to file the PR and add actually-structured bindings still!)
|
After merging your PR, Conbench analyzed the 5 benchmarking runs that have been run so far on merge-commit 22b42b0. There were no benchmark performance regressions. 🎉 The full Conbench report has more details. It also includes information about 1 possible false positive for unstable benchmarks that are known to sometimes produce them. |
…DERING and SQL_MAX_COLUMNS_IN_TABLE support to SqlInfoBuilder (apache#39561) This PR adds ability to specify `SQL_ALL_TABLES_ARE_SELECTABLE` and `SQL_NULL_ORDERING` metadata in `org.apache.arrow.flight.sql.SqlInfoBuilder`. ### Rationale for this change Without this change it is impossible to specify whether all tables are selectable, supported null ordering and maximum number of columns in table using `SqlInfoBuilder`. ### What changes are included in this PR? In this PR two methods are added to `SqlInfoBuilder`: - `withSqlAllTablesAreSelectable` accepting boolean parameter that specifies whether all tables are selectable - `withSqlNullOrdering` accepting `org.apache.arrow.flight.sql.impl.FlightSql.SqlNullOrdering` value that specifies supported null ordering - `withSqlMaxColumnsInTable` accepting long parameter that specifies maximum number of columns in table ### Are these changes tested? To ensure correctness `org.apache.arrow.flight.TestFlightSql#testGetSqlInfoResultsWithManyArgs` test is added). ### Are there any user-facing changes? This PR does not contain any breaking changes of user API. * Closes: apache#39558 Authored-by: Alexander Blazhkov <ablazhkov@querifylabs.com> Signed-off-by: David Li <li.davidm96@gmail.com>
…DERING and SQL_MAX_COLUMNS_IN_TABLE support to SqlInfoBuilder (apache#39561) This PR adds ability to specify `SQL_ALL_TABLES_ARE_SELECTABLE` and `SQL_NULL_ORDERING` metadata in `org.apache.arrow.flight.sql.SqlInfoBuilder`. ### Rationale for this change Without this change it is impossible to specify whether all tables are selectable, supported null ordering and maximum number of columns in table using `SqlInfoBuilder`. ### What changes are included in this PR? In this PR two methods are added to `SqlInfoBuilder`: - `withSqlAllTablesAreSelectable` accepting boolean parameter that specifies whether all tables are selectable - `withSqlNullOrdering` accepting `org.apache.arrow.flight.sql.impl.FlightSql.SqlNullOrdering` value that specifies supported null ordering - `withSqlMaxColumnsInTable` accepting long parameter that specifies maximum number of columns in table ### Are these changes tested? To ensure correctness `org.apache.arrow.flight.TestFlightSql#testGetSqlInfoResultsWithManyArgs` test is added). ### Are there any user-facing changes? This PR does not contain any breaking changes of user API. * Closes: apache#39558 Authored-by: Alexander Blazhkov <ablazhkov@querifylabs.com> Signed-off-by: David Li <li.davidm96@gmail.com>
This PR adds ability to specify
SQL_ALL_TABLES_ARE_SELECTABLEandSQL_NULL_ORDERINGmetadata inorg.apache.arrow.flight.sql.SqlInfoBuilder.Rationale for this change
Without this change it is impossible to specify whether all tables are selectable, supported null ordering and maximum number of columns in table using
SqlInfoBuilder.What changes are included in this PR?
In this PR two methods are added to
SqlInfoBuilder:withSqlAllTablesAreSelectableaccepting boolean parameter that specifies whether all tables are selectablewithSqlNullOrderingacceptingorg.apache.arrow.flight.sql.impl.FlightSql.SqlNullOrderingvalue that specifies supported null orderingwithSqlMaxColumnsInTableaccepting long parameter that specifies maximum number of columns in tableAre these changes tested?
To ensure correctness
org.apache.arrow.flight.TestFlightSql#testGetSqlInfoResultsWithManyArgstest is added).Are there any user-facing changes?
This PR does not contain any breaking changes of user API.