Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/23528 )
Change subject: IMPALA-12401: Support more info types for HS2 GetInfo() API ...................................................................... Patch Set 1: (15 comments) In general I think that these infos should be only filled when you are sure about there value. Some of them should be not filled by the server, as they provide info about the ODBC driver. http://gerrit.cloudera.org:8080/#/c/23528/1/be/src/service/impala-hs2-server.cc File be/src/service/impala-hs2-server.cc: http://gerrit.cloudera.org:8080/#/c/23528/1/be/src/service/impala-hs2-server.cc@587 PS1, Line 587: Adding a link to the definition of these values would be useful, e.g.: https://learn.microsoft.com/en-us/sql/odbc/reference/develop-app/driver-specific-data-types-descriptor-information-diagnostic?view=sql-server-ver17 http://gerrit.cloudera.org:8080/#/c/23528/1/be/src/service/impala-hs2-server.cc@618 PS1, Line 618: // SQL_UNICODE_STORED = 1 (case sensitive) The value should be SQL_IC_UPPER (2) based on https://learn.microsoft.com/en-us/sql/odbc/reference/develop-app/driver-specific-data-types-descriptor-information-diagnostic?view=sql-server-ver17 "SQL_IC_LOWER = Identifiers in SQL are not case-sensitive and are stored in lowercase in system catalog." This is exactly what happens in Impala + HMS http://gerrit.cloudera.org:8080/#/c/23528/1/be/src/service/impala-hs2-server.cc@622 PS1, Line 622: \" This is ` both in Hive and Impala, e.g. create table `select` (i int); http://gerrit.cloudera.org:8080/#/c/23528/1/be/src/service/impala-hs2-server.cc@638 PS1, Line 638: case TGetInfoType::CLI_CATALOG_NAME: : return_val.infoValue.__set_stringValue("default"); "SQL_CATALOG_NAME 3.0 A character string: "Y" if the server supports catalog names, or "N" if it does not. A SQL-92 Full level-conformant driver will always return "Y"." My guess is that this means whether Impala supports 3 level namespaces - the answer is no, but this is an actively developed area, probably it is better to leave it unfilled. http://gerrit.cloudera.org:8080/#/c/23528/1/be/src/service/impala-hs2-server.cc@645 PS1, Line 645: case TGetInfoType::CLI_MAX_COLUMNS_IN_SELECT: : return_val.infoValue.__set_lenValue(1000); : break; : case TGetInfoType::CLI_MAX_COLUMNS_IN_TABLE: : return_val.infoValue.__set_lenValue(1000); : break; : case TGetInfoType::CLI_MAX_COLUMNS_IN_GROUP_BY: : return_val.infoValue.__set_lenValue(1000); : break; : case TGetInfoType::CLI_MAX_COLUMNS_IN_ORDER_BY: : return_val.infoValue.__set_lenValue(1000); : break; : case TGetInfoType::CLI_MAX_TABLES_IN_SELECT: : return_val.infoValue.__set_lenValue(100); I don't know about these limits - do you have a source for them? http://gerrit.cloudera.org:8080/#/c/23528/1/be/src/service/impala-hs2-server.cc@660 PS1, Line 660: case TGetInfoType::CLI_MAX_STATEMENT_LEN: : return_val.infoValue.__set_lenValue(1048576); // 1MB : break; : case TGetInfoType::CLI_MAX_ROW_SIZE: : return_val.infoValue.__set_lenValue(1048576); // 1MB : break; These limits come from query options, could be deduced from the session https://github.com/apache/impala/blob/98f993da43f50a5dafdf0ae40795e93454814445/common/thrift/Query.thrift#L354 https://github.com/apache/impala/blob/98f993da43f50a5dafdf0ae40795e93454814445/common/thrift/Query.thrift#L463 http://gerrit.cloudera.org:8080/#/c/23528/1/be/src/service/impala-hs2-server.cc@682 PS1, Line 682: CLI_SCROLL_CONCURRENCY This is deprecated based on the docs, and I am also not sure about its value + whether it should be set by the driver or the server. http://gerrit.cloudera.org:8080/#/c/23528/1/be/src/service/impala-hs2-server.cc@690 PS1, Line 690: case TGetInfoType::CLI_GETDATA_EXTENSIONS: : // SQL_GD_ANY_COLUMN = 1, SQL_GD_ANY_ORDER = 2, SQL_GD_BOUND = 4 : return_val.infoValue.__set_integerBitmask(7); // All supported : break; This seems to be the property of the driver, not the server. http://gerrit.cloudera.org:8080/#/c/23528/1/be/src/service/impala-hs2-server.cc@702 PS1, Line 702: case TGetInfoType::CLI_CURSOR_SENSITIVITY: Impala doesn't have named cursors. http://gerrit.cloudera.org:8080/#/c/23528/1/be/src/service/impala-hs2-server.cc@707 PS1, Line 707: return_val.infoValue.__set_stringValue("UTF8"); Impala doesn't support collations at the moment. http://gerrit.cloudera.org:8080/#/c/23528/1/be/src/service/impala-hs2-server.cc@710 PS1, Line 710: return_val.infoValue.__set_stringValue("1995"); Some reference would be nice. http://gerrit.cloudera.org:8080/#/c/23528/1/be/src/service/impala-hs2-server.cc@712 PS1, Line 712: case TGetInfoType::CLI_MAX_DRIVER_CONNECTIONS: : return_val.infoValue.__set_smallIntValue(0); // No limit : break; : case TGetInfoType::CLI_MAX_CONCURRENT_ACTIVITIES: : return_val.infoValue.__set_smallIntValue(0); // No limit These are about the driver, not the server. http://gerrit.cloudera.org:8080/#/c/23528/1/be/src/service/impala-hs2-server.cc@721 PS1, Line 721: case TGetInfoType::CLI_ACCESSIBLE_TABLES: I am not sure here - I think that Impala will return a table if the user has any privilege, and that may not mean select/read privilege. http://gerrit.cloudera.org:8080/#/c/23528/1/be/src/service/impala-hs2-server.cc@741 PS1, Line 741: case TGetInfoType::CLI_MAX_COLUMNS_IN_INDEX: : return_val.infoValue.__set_lenValue(16); : break; : case TGetInfoType::CLI_MAX_INDEX_SIZE: : return_val.infoValue.__set_lenValue(1048576); // 1MB : break; Impala doesn't have indexes in general. http://gerrit.cloudera.org:8080/#/c/23528/1/common/thrift/hive-1-api/TCLIService.thrift File common/thrift/hive-1-api/TCLIService.thrift: http://gerrit.cloudera.org:8080/#/c/23528/1/common/thrift/hive-1-api/TCLIService.thrift@639 PS1, Line 639: CLI_ODBC_KEYWORDS = 10006, I would prefer to avoid changing TCLIService.thrift - ideally we should sync this time to time with Hive to avoid / minimize diverging. I am also unsure about the value of this, the Hive code also doesn't look correct to me. -- To view, visit http://gerrit.cloudera.org:8080/23528 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1ce5f2b9dcc2e4633b4679b002f57b5b4ea3e8bf Gerrit-Change-Number: 23528 Gerrit-PatchSet: 1 Gerrit-Owner: Arnab Karmakar <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Quanlong Huang <[email protected]> Gerrit-Comment-Date: Sun, 12 Oct 2025 09:25:19 +0000 Gerrit-HasComments: Yes
