Quanlong Huang 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 4:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/23528/4/be/src/service/impala-hs2-server.cc
File be/src/service/impala-hs2-server.cc:

http://gerrit.cloudera.org:8080/#/c/23528/4/be/src/service/impala-hs2-server.cc@116
PS4, Line 116: const string ODBC_KEYWORDS =
These are keywords of Transact-SQL used in SQL Server. Can we just use keywords 
in our sql-scanner?


http://gerrit.cloudera.org:8080/#/c/23528/4/be/src/service/impala-hs2-server.cc@598
PS4, Line 598: 767
In the initial patch set, we use 128 here which is consistent with Hive: 
https://github.com/apache/hive/blob/4903585a34ae44bb3fec4207b5acab63f6bfc8c1/service/src/java/org/apache/hive/service/cli/session/HiveSessionImpl.java#L502

Why do we change this to 767?


http://gerrit.cloudera.org:8080/#/c/23528/4/be/src/service/impala-hs2-server.cc@665
PS4, Line 665: int64_t
nit: should this be int32_t? max_statement_length_bytes is declared as i32 in 
thrift:
https://github.com/apache/impala/blob/f0a781806fa0bd2b2a4ab5af7f31f3bee4100654/common/thrift/Query.thrift#L463


http://gerrit.cloudera.org:8080/#/c/23528/4/be/src/service/impala-hs2-server.cc@682
PS4, Line 682: 524288
Can we get this from the query option, similar to what we do for 
'max_statement_length_bytes' ?


http://gerrit.cloudera.org:8080/#/c/23528/4/be/src/service/impala-hs2-server.cc@699
PS4, Line 699:       return_val.infoValue.__set_integerBitmask(1);
Impala also supports DROP COLUMN. Should we add SQL_AT_DROP_COLUMN as well? Are 
there any other bits that we should check?


http://gerrit.cloudera.org:8080/#/c/23528/4/tests/hs2/test_hs2.py
File tests/hs2/test_hs2.py:

http://gerrit.cloudera.org:8080/#/c/23528/4/tests/hs2/test_hs2.py@655
PS4, Line 655:  or
Why using "or" here? Will any of them fail?



--
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: 4
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: Thu, 16 Oct 2025 09:28:12 +0000
Gerrit-HasComments: Yes

Reply via email to