You are viewing a plain text version of this content. The canonical link for it is here.
Posted to gitbox@hive.apache.org by GitBox <gi...@apache.org> on 2022/04/30 13:53:02 UTC

[GitHub] [hive] zhangbutao opened a new pull request, #3260: HIVE-26192: JDBC data connector queries occur exception at cbo stage

zhangbutao opened a new pull request, #3260:
URL: https://github.com/apache/hive/pull/3260

   <!--
   Thanks for sending a pull request!  Here are some tips for you:
     1. If this is your first time, please read our contributor guidelines: https://cwiki.apache.org/confluence/display/Hive/HowToContribute
     2. Ensure that you have created an issue on the Hive project JIRA: https://issues.apache.org/jira/projects/HIVE/summary
     3. Ensure you have added or run the appropriate tests for your PR: 
     4. If the PR is unfinished, add '[WIP]' in your PR title, e.g., '[WIP]HIVE-XXXXX:  Your PR title ...'.
     5. Be sure to keep the PR description updated to reflect all changes.
     6. Please write your PR title to summarize what this PR proposes.
     7. If possible, provide a concise example to reproduce the issue for a faster review.
   
   -->
   
   ### What changes were proposed in this pull request?
   <!--
   Please clarify what changes you are proposing. The purpose of this section is to outline the changes and how this PR fixes the issue. 
   If possible, please consider writing useful notes for better and faster reviews in your PR. See the examples below.
     1. If you refactor some codes with changing classes, showing the class hierarchy will help reviewers.
     2. If you fix some SQL features, you can provide some references of other DBMSes.
     3. If there is design documentation, please add the link.
     4. If there is a discussion in the mailing list, please add the link.
   -->
   Change `hive.sql.table`  tblproperty value from database.tablename to tablename.
   
   ### Why are the changes needed?
   <!--
   Please clarify why the changes are needed. For instance,
     1. If you propose a new API, clarify the use case for a new API.
     2. If you fix a bug, you can clarify why it is a bug.
   -->
   Plead see error info from HIVE-26192;
   CBO stage should get jdbc table info by using tablename instead of database.tablename.
   
   ### Does this PR introduce _any_ user-facing change?
   <!--
   Note that it means *any* user-facing change including all aspects such as the documentation fix.
   If yes, please clarify the previous behavior and the change this PR proposes - provide the console output, description, screenshot and/or a reproducable example to show the behavior difference if possible.
   If possible, please also clarify if this is a user-facing change compared to the released Hive versions or within the unreleased branches such as master.
   If no, write 'No'.
   -->
   No
   
   ### How was this patch tested?
   <!--
   If tests were added, say they were added here. Please make sure to add some test cases that check the changes thoroughly including negative and positive cases if possible.
   If it was tested in a way different from regular unit tests, please clarify how you tested step by step, ideally copy and paste-able, so that other reviewers can test and check, and descendants can verify in the future.
   If tests were not added, please describe why they were not added and/or why it was difficult to add.
   -->
   mvn test -Dtest=TestMiniLlapCliDriver -Dqfile=dataconnector_mysql.q -Dtest.output.overwrite=true
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org


[GitHub] [hive] zhangbutao commented on pull request #3260: HIVE-26192: JDBC data connector queries occur exception at cbo stage

Posted by GitBox <gi...@apache.org>.
zhangbutao commented on PR #3260:
URL: https://github.com/apache/hive/pull/3260#issuecomment-1113993024

   @nrg4878 Could you take a look when you are free. thx.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org


[GitHub] [hive] zhangbutao commented on a diff in pull request #3260: HIVE-26192: JDBC data connector queries occur exception at cbo stage

Posted by GitBox <gi...@apache.org>.
zhangbutao commented on code in PR #3260:
URL: https://github.com/apache/hive/pull/3260#discussion_r873641385


##########
ql/src/test/queries/clientpositive/dataconnector_mysql.q:
##########
@@ -14,6 +14,9 @@ SHOW DATABASES;
 USE db_mysql;
 SHOW TABLES;
 
+SHOW CREATE TABLE country;
+SELECT * FROM country;

Review Comment:
   > Can you add similar tests for the other supported connectors? (Oracle, MSSQL, etc.). I would like to ensure that we are not breaking something else while fixing this problem.
   
   Adding qtest for Oracle or MSSQL needs docker env like MySQL qtest. This test may be outside the scope of this pr but make sense. 
   But i think we should optimize this fix to only modify method of subclasses(MySQL, Oracle, MSSQL) instead of modifing parent class ` AbstractJDBCConnectorProvider.java` repeatedly. Because it is difficult to ensure that all jdbc connectors have consistent table structures.
    wdyt. thx. @zabetak 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org


[GitHub] [hive] nrg4878 merged pull request #3260: HIVE-26192: JDBC data connector queries occur exception at cbo stage

Posted by GitBox <gi...@apache.org>.
nrg4878 merged PR #3260:
URL: https://github.com/apache/hive/pull/3260


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org


[GitHub] [hive] zabetak commented on a diff in pull request #3260: HIVE-26192: JDBC data connector queries occur exception at cbo stage

Posted by GitBox <gi...@apache.org>.
zabetak commented on code in PR #3260:
URL: https://github.com/apache/hive/pull/3260#discussion_r873683117


##########
ql/src/test/queries/clientpositive/dataconnector_mysql.q:
##########
@@ -14,6 +14,9 @@ SHOW DATABASES;
 USE db_mysql;
 SHOW TABLES;
 
+SHOW CREATE TABLE country;
+SELECT * FROM country;

Review Comment:
   Ideally the tests should be present in the same commit that fixed HIVE-25091. It is a bit unfortunate that HIVE-25091 didn't add any tests. 
   
   I cannot tell for sure if the best way to handle this kind of problem is through sub-classing cause the specs/docs for the CONNECTOR are somewhat incomplete and tests are almost nonexistent.
   
   I agree with you that tests are not necessarily in the scope of this PR. However, for moving forward with this or other fixes we should have a minimal set of tests in place which at least cover the basic functionality. 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org


[GitHub] [hive] nrg4878 commented on pull request #3260: HIVE-26192: JDBC data connector queries occur exception at cbo stage

Posted by GitBox <gi...@apache.org>.
nrg4878 commented on PR #3260:
URL: https://github.com/apache/hive/pull/3260#issuecomment-1188547169

   @zabetak @zhangbutao is this ready to be committed? Looks like a test has been added as well


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org


[GitHub] [hive] zabetak commented on a diff in pull request #3260: HIVE-26192: JDBC data connector queries occur exception at cbo stage

Posted by GitBox <gi...@apache.org>.
zabetak commented on code in PR #3260:
URL: https://github.com/apache/hive/pull/3260#discussion_r873567240


##########
ql/src/test/queries/clientpositive/dataconnector_mysql.q:
##########
@@ -14,6 +14,9 @@ SHOW DATABASES;
 USE db_mysql;
 SHOW TABLES;
 
+SHOW CREATE TABLE country;
+SELECT * FROM country;

Review Comment:
   Can you add similar tests for the other supported connectors? (Oracle, MSSQL, etc.). I would like to ensure that we are not breaking something else while fixing this problem.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org


[GitHub] [hive] github-actions[bot] commented on pull request #3260: HIVE-26192: JDBC data connector queries occur exception at cbo stage

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #3260:
URL: https://github.com/apache/hive/pull/3260#issuecomment-1186036434

   This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.
   Feel free to reach out on the dev@hive.apache.org list if the patch is in need of reviews.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org


[GitHub] [hive] zhangbutao commented on pull request #3260: HIVE-26192: JDBC data connector queries occur exception at cbo stage

Posted by GitBox <gi...@apache.org>.
zhangbutao commented on PR #3260:
URL: https://github.com/apache/hive/pull/3260#issuecomment-1188553915

   
   > @zabetak @zhangbutao is this ready to be committed? Looks like a test has been added as well
   
   @nrg4878 I have test this change with MySQL connector. But other JDBC connectors (Oracle, MSSQL) which introduced by HIVE-25091 have not tested yet,  because the two connectors do not have docker integration tests in current code.
   I think we can merge this PR to fix regression from HIVE-25091 first, and then add docker integration tests for other JDBC connectors. Thx.
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org