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 2021/09/29 22:03:27 UTC

[GitHub] [hive] soumyakanti3578 opened a new pull request #2685: Hive 25530: AssertionError when query involves multiple JDBC tables and views

soumyakanti3578 opened a new pull request #2685:
URL: https://github.com/apache/hive/pull/2685


   <!--
   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?
   Small change to ensure `JdbcConvention` and `JdbcSchema` objects are not created multiple times in a query.
   
   
   ### Why are the changes needed?
   These changes prevent adding same Jdbc rule twice and throwing an Assertion error.
   
   
   ### Does this PR introduce _any_ user-facing change?
   No
   
   
   ### How was this patch tested?
   `mvn test -Dtest=TestMiniLlapLocalCliDriver -Dtest.output.overwrite=true -Dqfile=external_jdbc_join_mv.q`
   


-- 
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] kasakrisz commented on a change in pull request #2685: Hive 25530: AssertionError when query involves multiple JDBC tables and views

Posted by GitBox <gi...@apache.org>.
kasakrisz commented on a change in pull request #2685:
URL: https://github.com/apache/hive/pull/2685#discussion_r727987446



##########
File path: ql/src/test/queries/clientpositive/external_jdbc_join_mv.q
##########
@@ -0,0 +1,60 @@
+set hive.support.concurrency=true;
+set hive.txn.manager=org.apache.hadoop.hive.ql.lockmgr.DbTxnManager;
+
+CREATE TEMPORARY FUNCTION dboutput AS 'org.apache.hadoop.hive.contrib.genericudf.example.GenericUDFDBOutput';
+
+SELECT
+dboutput ( 'jdbc:derby:;databaseName=${system:test.tmp.dir}/test_jdbc_join_mv;create=true','','',
+'CREATE TABLE person ("id" INTEGER, "name" VARCHAR(25), "jid" INTEGER, "cid" INTEGER)' );
+
+SELECT
+dboutput ( 'jdbc:derby:;databaseName=${system:test.tmp.dir}/test_jdbc_join_mv;create=true','','',
+'CREATE TABLE country ("id" INTEGER, "name" VARCHAR(25))' );
+
+CREATE EXTERNAL TABLE person
+(
+ id int,
+ name varchar(25),
+ jid int,
+ cid int
+)
+STORED BY 'org.apache.hive.storage.jdbc.JdbcStorageHandler'
+TBLPROPERTIES (
+                "hive.sql.database.type" = "DERBY",
+                "hive.sql.jdbc.driver" = "org.apache.derby.jdbc.EmbeddedDriver",
+                "hive.sql.jdbc.url" = "jdbc:derby:;databaseName=${system:test.tmp.dir}/test_jdbc_join_mv;create=true;collation=TERRITORY_BASED:PRIMARY",
+                "hive.sql.dbcp.username" = "APP",
+                "hive.sql.dbcp.password" = "mine",
+                "hive.sql.table" = "PERSON",
+                "hive.sql.dbcp.maxActive" = "1"
+);
+
+CREATE EXTERNAL TABLE country
+(
+    id int,
+    name varchar(25)
+)
+    STORED BY 'org.apache.hive.storage.jdbc.JdbcStorageHandler'
+    TBLPROPERTIES (
+        "hive.sql.database.type" = "DERBY",
+        "hive.sql.jdbc.driver" = "org.apache.derby.jdbc.EmbeddedDriver",
+        "hive.sql.jdbc.url" = "jdbc:derby:;databaseName=${system:test.tmp.dir}/test_jdbc_join_mv;create=true;collation=TERRITORY_BASED:PRIMARY",
+        "hive.sql.dbcp.username" = "APP",
+        "hive.sql.dbcp.password" = "mine",
+        "hive.sql.table" = "COUNTRY",
+        "hive.sql.dbcp.maxActive" = "1"
+        );
+
+CREATE TABLE job (
+    id int,
+    title varchar(20)
+) STORED AS ORC TBLPROPERTIES ('transactional'='true');
+
+CREATE MATERIALIZED VIEW mv1 AS SELECT id, title FROM job WHERE title = 'Software Engineer';
+
+explain cbo 
+select * 
+from person 
+join job on person.jid = job.id
+join country on person.cid = country.id
+where job.title = 'Software Engineer';

Review comment:
       Please drop the materialized view in the end of the test.
   MVs plan are cached in memory in HS2 without any time constraint and it can affect other test cases when running in batch.




-- 
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] kasakrisz commented on pull request #2685: Hive 25530: AssertionError when query involves multiple JDBC tables and views

Posted by GitBox <gi...@apache.org>.
kasakrisz commented on pull request #2685:
URL: https://github.com/apache/hive/pull/2685#issuecomment-942241560


   Could you please fix the jira ticket id format in the commit message like: `HIVE-25530: AssertionError...`
   https://cwiki.apache.org/confluence/display/Hive/HowToContribute#HowToContribute-SubmittingaPR


-- 
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] asolimando commented on a change in pull request #2685: Hive 25530: AssertionError when query involves multiple JDBC tables and views

Posted by GitBox <gi...@apache.org>.
asolimando commented on a change in pull request #2685:
URL: https://github.com/apache/hive/pull/2685#discussion_r726834808



##########
File path: ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java
##########
@@ -3021,11 +3026,24 @@ private RelNode genTableLogicalPlan(String tableAlias, QB qb) throws SemanticExc
 
             DataSource ds = JdbcSchema.dataSource(url, driver, user, pswd);
             SqlDialect jdbcDialect = JdbcSchema.createDialect(SqlDialectFactoryImpl.INSTANCE, ds);
+            String dialectName = jdbcDialect.getClass().getName();
             if (LOG.isDebugEnabled()) {
-              LOG.debug("Dialect for table {}: {}", tableName, jdbcDialect.getClass().getName());
+              LOG.debug("Dialect for table {}: {}", tableName, dialectName);
+            }
+
+            List<String> jcKey = ImmutableNullableList.of(url, driver, user, pswd, dialectName, dataBaseType);
+            if (!jcMap.containsKey(jcKey)) {
+              jcMap.put(jcKey, JdbcConvention.of(jdbcDialect, null, dataBaseType));
             }
-            JdbcConvention jc = JdbcConvention.of(jdbcDialect, null, dataBaseType);
-            JdbcSchema schema = new JdbcSchema(ds, jc.dialect, jc, catalogName, schemaName);
+            JdbcConvention jc = jcMap.get(jcKey);
+
+            List<String> schemaKey = ImmutableNullableList.of(url, driver, user, pswd, dialectName, dataBaseType,
+              catalogName, schemaName);
+            if (!schemaMap.containsKey(schemaKey)) {

Review comment:
       Nit: replace "if + put" with `Map.putIfAbsent`?




-- 
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] soumyakanti3578 commented on a change in pull request #2685: Hive 25530: AssertionError when query involves multiple JDBC tables and views

Posted by GitBox <gi...@apache.org>.
soumyakanti3578 commented on a change in pull request #2685:
URL: https://github.com/apache/hive/pull/2685#discussion_r727271186



##########
File path: ql/src/test/queries/clientpositive/external_jdbc_join_mv.q
##########
@@ -0,0 +1,60 @@
+set hive.support.concurrency=true;
+set hive.txn.manager=org.apache.hadoop.hive.ql.lockmgr.DbTxnManager;
+
+CREATE TEMPORARY FUNCTION dboutput AS 'org.apache.hadoop.hive.contrib.genericudf.example.GenericUDFDBOutput';
+
+SELECT
+dboutput ( 'jdbc:derby:;databaseName=${system:test.tmp.dir}/engesc_6056;create=true','','',
+'CREATE TABLE person ("id" INTEGER, "name" VARCHAR(25), "jid" INTEGER, "cid" INTEGER)' );
+
+SELECT
+dboutput ( 'jdbc:derby:;databaseName=${system:test.tmp.dir}/engesc_6056;create=true','','',
+'CREATE TABLE country ("id" INTEGER, "name" VARCHAR(25))' );
+
+CREATE EXTERNAL TABLE person
+(
+ id int,
+ name varchar(25),
+ jid int,
+ cid int
+)
+STORED BY 'org.apache.hive.storage.jdbc.JdbcStorageHandler'
+TBLPROPERTIES (
+                "hive.sql.database.type" = "DERBY",
+                "hive.sql.jdbc.driver" = "org.apache.derby.jdbc.EmbeddedDriver",
+                "hive.sql.jdbc.url" = "jdbc:derby:;databaseName=${system:test.tmp.dir}/engesc_6056;create=true;collation=TERRITORY_BASED:PRIMARY",

Review comment:
       Totally missed this one! Thanks for catching it 👍🏼 




-- 
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] asolimando commented on a change in pull request #2685: Hive 25530: AssertionError when query involves multiple JDBC tables and views

Posted by GitBox <gi...@apache.org>.
asolimando commented on a change in pull request #2685:
URL: https://github.com/apache/hive/pull/2685#discussion_r726828332



##########
File path: ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java
##########
@@ -1612,6 +1613,10 @@ private RowResolver genRowResolver(Operator op, QB qb) {
     private final StatsSource statsSource;
     private RelNode dummyTableScan;
 
+    // these variables are to ensure that they get instantiated only once
+    Map<List<String>, JdbcConvention> jcMap = new HashMap<>();

Review comment:
       Nit: nowadays with autocompletion I think we can afford longer and self-explanatory variable names like `jdbcConventionMap`, WDYT?




-- 
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] kasakrisz commented on a change in pull request #2685: Hive 25530: AssertionError when query involves multiple JDBC tables and views

Posted by GitBox <gi...@apache.org>.
kasakrisz commented on a change in pull request #2685:
URL: https://github.com/apache/hive/pull/2685#discussion_r727987446



##########
File path: ql/src/test/queries/clientpositive/external_jdbc_join_mv.q
##########
@@ -0,0 +1,60 @@
+set hive.support.concurrency=true;
+set hive.txn.manager=org.apache.hadoop.hive.ql.lockmgr.DbTxnManager;
+
+CREATE TEMPORARY FUNCTION dboutput AS 'org.apache.hadoop.hive.contrib.genericudf.example.GenericUDFDBOutput';
+
+SELECT
+dboutput ( 'jdbc:derby:;databaseName=${system:test.tmp.dir}/test_jdbc_join_mv;create=true','','',
+'CREATE TABLE person ("id" INTEGER, "name" VARCHAR(25), "jid" INTEGER, "cid" INTEGER)' );
+
+SELECT
+dboutput ( 'jdbc:derby:;databaseName=${system:test.tmp.dir}/test_jdbc_join_mv;create=true','','',
+'CREATE TABLE country ("id" INTEGER, "name" VARCHAR(25))' );
+
+CREATE EXTERNAL TABLE person
+(
+ id int,
+ name varchar(25),
+ jid int,
+ cid int
+)
+STORED BY 'org.apache.hive.storage.jdbc.JdbcStorageHandler'
+TBLPROPERTIES (
+                "hive.sql.database.type" = "DERBY",
+                "hive.sql.jdbc.driver" = "org.apache.derby.jdbc.EmbeddedDriver",
+                "hive.sql.jdbc.url" = "jdbc:derby:;databaseName=${system:test.tmp.dir}/test_jdbc_join_mv;create=true;collation=TERRITORY_BASED:PRIMARY",
+                "hive.sql.dbcp.username" = "APP",
+                "hive.sql.dbcp.password" = "mine",
+                "hive.sql.table" = "PERSON",
+                "hive.sql.dbcp.maxActive" = "1"
+);
+
+CREATE EXTERNAL TABLE country
+(
+    id int,
+    name varchar(25)
+)
+    STORED BY 'org.apache.hive.storage.jdbc.JdbcStorageHandler'
+    TBLPROPERTIES (
+        "hive.sql.database.type" = "DERBY",
+        "hive.sql.jdbc.driver" = "org.apache.derby.jdbc.EmbeddedDriver",
+        "hive.sql.jdbc.url" = "jdbc:derby:;databaseName=${system:test.tmp.dir}/test_jdbc_join_mv;create=true;collation=TERRITORY_BASED:PRIMARY",
+        "hive.sql.dbcp.username" = "APP",
+        "hive.sql.dbcp.password" = "mine",
+        "hive.sql.table" = "COUNTRY",
+        "hive.sql.dbcp.maxActive" = "1"
+        );
+
+CREATE TABLE job (
+    id int,
+    title varchar(20)
+) STORED AS ORC TBLPROPERTIES ('transactional'='true');
+
+CREATE MATERIALIZED VIEW mv1 AS SELECT id, title FROM job WHERE title = 'Software Engineer';
+
+explain cbo 
+select * 
+from person 
+join job on person.jid = job.id
+join country on person.cid = country.id
+where job.title = 'Software Engineer';

Review comment:
       Please drop the materialized view in the end of the test.
   MVs plan are cached in memory in HS2 without any time constraint and it can affect other test cases when running in batch.




-- 
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] asolimando commented on a change in pull request #2685: Hive 25530: AssertionError when query involves multiple JDBC tables and views

Posted by GitBox <gi...@apache.org>.
asolimando commented on a change in pull request #2685:
URL: https://github.com/apache/hive/pull/2685#discussion_r726835896



##########
File path: ql/src/test/queries/clientpositive/external_jdbc_join_mv.q
##########
@@ -0,0 +1,60 @@
+set hive.support.concurrency=true;
+set hive.txn.manager=org.apache.hadoop.hive.ql.lockmgr.DbTxnManager;
+
+CREATE TEMPORARY FUNCTION dboutput AS 'org.apache.hadoop.hive.contrib.genericudf.example.GenericUDFDBOutput';
+
+SELECT
+dboutput ( 'jdbc:derby:;databaseName=${system:test.tmp.dir}/engesc_6056;create=true','','',
+'CREATE TABLE person ("id" INTEGER, "name" VARCHAR(25), "jid" INTEGER, "cid" INTEGER)' );
+
+SELECT
+dboutput ( 'jdbc:derby:;databaseName=${system:test.tmp.dir}/engesc_6056;create=true','','',
+'CREATE TABLE country ("id" INTEGER, "name" VARCHAR(25))' );
+
+CREATE EXTERNAL TABLE person
+(
+ id int,
+ name varchar(25),
+ jid int,
+ cid int
+)
+STORED BY 'org.apache.hive.storage.jdbc.JdbcStorageHandler'
+TBLPROPERTIES (
+                "hive.sql.database.type" = "DERBY",
+                "hive.sql.jdbc.driver" = "org.apache.derby.jdbc.EmbeddedDriver",
+                "hive.sql.jdbc.url" = "jdbc:derby:;databaseName=${system:test.tmp.dir}/engesc_6056;create=true;collation=TERRITORY_BASED:PRIMARY",
+                "hive.sql.dbcp.username" = "APP",
+                "hive.sql.dbcp.password" = "mine",
+                "hive.sql.table" = "PERSON",
+                "hive.sql.dbcp.maxActive" = "1"
+);
+
+CREATE EXTERNAL TABLE country
+(
+    id int,
+    name varchar(25)
+)
+    STORED BY 'org.apache.hive.storage.jdbc.JdbcStorageHandler'
+    TBLPROPERTIES (
+        "hive.sql.database.type" = "DERBY",
+        "hive.sql.jdbc.driver" = "org.apache.derby.jdbc.EmbeddedDriver",
+        "hive.sql.jdbc.url" = "jdbc:derby:;databaseName=${system:test.tmp.dir}/engesc_6056;create=true;collation=TERRITORY_BASED:PRIMARY",
+        "hive.sql.dbcp.username" = "APP",
+        "hive.sql.dbcp.password" = "mine",
+        "hive.sql.table" = "COUNTRY",
+        "hive.sql.dbcp.maxActive" = "1"
+        );
+
+CREATE TABLE job (
+    id int,
+    title varchar(20)
+) STORED AS ORC TBLPROPERTIES ('transactional'='true');
+
+CREATE MATERIALIZED VIEW mv1 AS SELECT id, title FROM job WHERE title = 'Software Engineer';
+
+explain cbo 
+select * 
+from person 
+join job on person.jid = job.id
+join country on person.cid = country.id
+where job.title = 'Software Engineer';

Review comment:
       Text files should always end with a newline




-- 
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] soumyakanti3578 commented on a change in pull request #2685: Hive 25530: AssertionError when query involves multiple JDBC tables and views

Posted by GitBox <gi...@apache.org>.
soumyakanti3578 commented on a change in pull request #2685:
URL: https://github.com/apache/hive/pull/2685#discussion_r727271186



##########
File path: ql/src/test/queries/clientpositive/external_jdbc_join_mv.q
##########
@@ -0,0 +1,60 @@
+set hive.support.concurrency=true;
+set hive.txn.manager=org.apache.hadoop.hive.ql.lockmgr.DbTxnManager;
+
+CREATE TEMPORARY FUNCTION dboutput AS 'org.apache.hadoop.hive.contrib.genericudf.example.GenericUDFDBOutput';
+
+SELECT
+dboutput ( 'jdbc:derby:;databaseName=${system:test.tmp.dir}/engesc_6056;create=true','','',
+'CREATE TABLE person ("id" INTEGER, "name" VARCHAR(25), "jid" INTEGER, "cid" INTEGER)' );
+
+SELECT
+dboutput ( 'jdbc:derby:;databaseName=${system:test.tmp.dir}/engesc_6056;create=true','','',
+'CREATE TABLE country ("id" INTEGER, "name" VARCHAR(25))' );
+
+CREATE EXTERNAL TABLE person
+(
+ id int,
+ name varchar(25),
+ jid int,
+ cid int
+)
+STORED BY 'org.apache.hive.storage.jdbc.JdbcStorageHandler'
+TBLPROPERTIES (
+                "hive.sql.database.type" = "DERBY",
+                "hive.sql.jdbc.driver" = "org.apache.derby.jdbc.EmbeddedDriver",
+                "hive.sql.jdbc.url" = "jdbc:derby:;databaseName=${system:test.tmp.dir}/engesc_6056;create=true;collation=TERRITORY_BASED:PRIMARY",

Review comment:
       Totally missed this one! Thanks for catching it 👍🏼 




-- 
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] kasakrisz merged pull request #2685: HIVE-25530: AssertionError when query involves multiple JDBC tables and views

Posted by GitBox <gi...@apache.org>.
kasakrisz merged pull request #2685:
URL: https://github.com/apache/hive/pull/2685


   


-- 
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] kasakrisz commented on pull request #2685: Hive 25530: AssertionError when query involves multiple JDBC tables and views

Posted by GitBox <gi...@apache.org>.
kasakrisz commented on pull request #2685:
URL: https://github.com/apache/hive/pull/2685#issuecomment-942241560


   Could you please fix the jira ticket id format in the commit message like: `HIVE-25530: AssertionError...`
   https://cwiki.apache.org/confluence/display/Hive/HowToContribute#HowToContribute-SubmittingaPR


-- 
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] asolimando commented on a change in pull request #2685: Hive 25530: AssertionError when query involves multiple JDBC tables and views

Posted by GitBox <gi...@apache.org>.
asolimando commented on a change in pull request #2685:
URL: https://github.com/apache/hive/pull/2685#discussion_r727233400



##########
File path: ql/src/test/queries/clientpositive/external_jdbc_join_mv.q
##########
@@ -0,0 +1,60 @@
+set hive.support.concurrency=true;
+set hive.txn.manager=org.apache.hadoop.hive.ql.lockmgr.DbTxnManager;
+
+CREATE TEMPORARY FUNCTION dboutput AS 'org.apache.hadoop.hive.contrib.genericudf.example.GenericUDFDBOutput';
+
+SELECT
+dboutput ( 'jdbc:derby:;databaseName=${system:test.tmp.dir}/engesc_6056;create=true','','',
+'CREATE TABLE person ("id" INTEGER, "name" VARCHAR(25), "jid" INTEGER, "cid" INTEGER)' );
+
+SELECT
+dboutput ( 'jdbc:derby:;databaseName=${system:test.tmp.dir}/engesc_6056;create=true','','',
+'CREATE TABLE country ("id" INTEGER, "name" VARCHAR(25))' );
+
+CREATE EXTERNAL TABLE person
+(
+ id int,
+ name varchar(25),
+ jid int,
+ cid int
+)
+STORED BY 'org.apache.hive.storage.jdbc.JdbcStorageHandler'
+TBLPROPERTIES (
+                "hive.sql.database.type" = "DERBY",
+                "hive.sql.jdbc.driver" = "org.apache.derby.jdbc.EmbeddedDriver",
+                "hive.sql.jdbc.url" = "jdbc:derby:;databaseName=${system:test.tmp.dir}/engesc_6056;create=true;collation=TERRITORY_BASED:PRIMARY",

Review comment:
       If needed let's replace `engesc_6056` with `hive_25530`, but we probably don't need to avoid collisions and we can simply go for "db" or similar.




-- 
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] asolimando commented on a change in pull request #2685: Hive 25530: AssertionError when query involves multiple JDBC tables and views

Posted by GitBox <gi...@apache.org>.
asolimando commented on a change in pull request #2685:
URL: https://github.com/apache/hive/pull/2685#discussion_r726828865



##########
File path: ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java
##########
@@ -3021,11 +3026,24 @@ private RelNode genTableLogicalPlan(String tableAlias, QB qb) throws SemanticExc
 
             DataSource ds = JdbcSchema.dataSource(url, driver, user, pswd);
             SqlDialect jdbcDialect = JdbcSchema.createDialect(SqlDialectFactoryImpl.INSTANCE, ds);
+            String dialectName = jdbcDialect.getClass().getName();
             if (LOG.isDebugEnabled()) {
-              LOG.debug("Dialect for table {}: {}", tableName, jdbcDialect.getClass().getName());
+              LOG.debug("Dialect for table {}: {}", tableName, dialectName);
+            }
+
+            List<String> jcKey = ImmutableNullableList.of(url, driver, user, pswd, dialectName, dataBaseType);
+            if (!jcMap.containsKey(jcKey)) {
+              jcMap.put(jcKey, JdbcConvention.of(jdbcDialect, null, dataBaseType));
             }
-            JdbcConvention jc = JdbcConvention.of(jdbcDialect, null, dataBaseType);
-            JdbcSchema schema = new JdbcSchema(ds, jc.dialect, jc, catalogName, schemaName);
+            JdbcConvention jc = jcMap.get(jcKey);

Review comment:
       Nit: same comment about longer names




-- 
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