You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by GitBox <gi...@apache.org> on 2021/04/16 07:58:58 UTC
[GitHub] [calcite] NobiGo opened a new pull request #2402: [CALCITE-4585]when run RelNode or Queryable,Confuse log info be thrown
NobiGo opened a new pull request #2402:
URL: https://github.com/apache/calcite/pull/2402
This code capture the exception,But Only the output of SQL exception is processed.So need to add the procedure to handle extra exception.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] danny0405 commented on a change in pull request #2402: [CALCITE-4585]when run RelNode or Queryable,Confuse log info be thrown(xiong duan)
Posted by GitBox <gi...@apache.org>.
danny0405 commented on a change in pull request #2402:
URL: https://github.com/apache/calcite/pull/2402#discussion_r618989740
##########
File path: core/src/test/java/org/apache/calcite/test/ExceptionMessageTest.java
##########
@@ -141,4 +167,29 @@ private void runQuery(String sql) throws SQLException {
containsString("Object 'nonexistentTable' not found"));
}
}
+
+ @Test void testValidRelNodeQuery() throws SQLException {
+ final RelNode relNode = builder
+ .scan("test", "entries")
+ .project(builder.field("name"))
+ .build();
+ runQuery(relNode);
+ }
+
+ @Test void testRelNodeQueryException() throws SQLException {
+ try {
+ final RelNode relNode = builder
+ .scan("test", "entries")
+ .project(builder.call(SqlStdOperatorTable.ABS, builder.field("name")))
+ .build();
+ runQuery(relNode);
+ fail("Query badEntries should result in an exception");
+ } catch (RuntimeException e) {
+ assertThat(e.getMessage(),
+ equalTo("java.sql.SQLException: Error while preparing statement [\n"
+ + "LogicalProject($f0=[ABS($1)])\n"
Review comment:
Hmm, the plan may be helpful but it is not a statement ?
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] Aaaaaaron commented on a change in pull request #2402: [CALCITE-4585]when run RelNode or Queryable,Confuse log info be thrown(xiong duan)
Posted by GitBox <gi...@apache.org>.
Aaaaaaron commented on a change in pull request #2402:
URL: https://github.com/apache/calcite/pull/2402#discussion_r619939520
##########
File path: core/src/test/java/org/apache/calcite/test/ExceptionMessageTest.java
##########
@@ -141,4 +167,29 @@ private void runQuery(String sql) throws SQLException {
containsString("Object 'nonexistentTable' not found"));
}
}
+
+ @Test void testValidRelNodeQuery() throws SQLException {
+ final RelNode relNode = builder
+ .scan("test", "entries")
+ .project(builder.field("name"))
+ .build();
+ runQuery(relNode);
+ }
+
+ @Test void testRelNodeQueryException() throws SQLException {
+ try {
+ final RelNode relNode = builder
+ .scan("test", "entries")
+ .project(builder.call(SqlStdOperatorTable.ABS, builder.field("name")))
+ .build();
+ runQuery(relNode);
+ fail("Query badEntries should result in an exception");
+ } catch (RuntimeException e) {
+ assertThat(e.getMessage(),
+ equalTo("java.sql.SQLException: Error while preparing statement [\n"
+ + "LogicalProject($f0=[ABS($1)])\n"
Review comment:
Actually, I'm +1 for Danny, maybe change "statement" to "plan" is better. But the code before is also "Error while preparing statement"
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] julianhyde commented on pull request #2402: [CALCITE-4585]when run RelNode or Queryable,Confuse log info be thrown(xiong duan)
Posted by GitBox <gi...@apache.org>.
julianhyde commented on pull request #2402:
URL: https://github.com/apache/calcite/pull/2402#issuecomment-826599546
We don't generally use `System.lineSeparator()`; we generally use "\n" in strings. To make the tests pass in Windows, use `Matchers.isLinux` in your test.
The commit message (and JIRA subject) is not clear. Can you improve it?
I don't think it will be clear to future maintainers what is the purpose of the two test methods. Please add javadoc comments to those methods indicating what you are testing.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] NobiGo commented on a change in pull request #2402: [CALCITE-4585]when run RelNode or Queryable,Confuse log info be thrown(xiong duan)
Posted by GitBox <gi...@apache.org>.
NobiGo commented on a change in pull request #2402:
URL: https://github.com/apache/calcite/pull/2402#discussion_r619004512
##########
File path: core/src/test/java/org/apache/calcite/test/ExceptionMessageTest.java
##########
@@ -141,4 +167,29 @@ private void runQuery(String sql) throws SQLException {
containsString("Object 'nonexistentTable' not found"));
}
}
+
+ @Test void testValidRelNodeQuery() throws SQLException {
+ final RelNode relNode = builder
+ .scan("test", "entries")
+ .project(builder.field("name"))
+ .build();
+ runQuery(relNode);
+ }
+
+ @Test void testRelNodeQueryException() throws SQLException {
+ try {
+ final RelNode relNode = builder
+ .scan("test", "entries")
+ .project(builder.call(SqlStdOperatorTable.ABS, builder.field("name")))
+ .build();
+ runQuery(relNode);
+ fail("Query badEntries should result in an exception");
+ } catch (RuntimeException e) {
+ assertThat(e.getMessage(),
+ equalTo("java.sql.SQLException: Error while preparing statement [\n"
+ + "LogicalProject($f0=[ABS($1)])\n"
Review comment:
Statement in Calcite may be include three types: Sql. RelNode . Queryable. This method Called prepareStatement_. My be we listen to others opinion.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] NobiGo commented on a change in pull request #2402: [CALCITE-4585]when run RelNode or Queryable,Confuse log info be thrown(xiong duan)
Posted by GitBox <gi...@apache.org>.
NobiGo commented on a change in pull request #2402:
URL: https://github.com/apache/calcite/pull/2402#discussion_r619986247
##########
File path: core/src/test/java/org/apache/calcite/test/ExceptionMessageTest.java
##########
@@ -141,4 +167,29 @@ private void runQuery(String sql) throws SQLException {
containsString("Object 'nonexistentTable' not found"));
}
}
+
+ @Test void testValidRelNodeQuery() throws SQLException {
+ final RelNode relNode = builder
+ .scan("test", "entries")
+ .project(builder.field("name"))
+ .build();
+ runQuery(relNode);
+ }
+
+ @Test void testRelNodeQueryException() throws SQLException {
+ try {
+ final RelNode relNode = builder
+ .scan("test", "entries")
+ .project(builder.call(SqlStdOperatorTable.ABS, builder.field("name")))
+ .build();
+ runQuery(relNode);
+ fail("Query badEntries should result in an exception");
+ } catch (RuntimeException e) {
+ assertThat(e.getMessage(),
+ equalTo("java.sql.SQLException: Error while preparing statement [\n"
+ + "LogicalProject($f0=[ABS($1)])\n"
Review comment:
Ok. I will change this to plan. Thanks for all of your advice.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] NobiGo commented on a change in pull request #2402: [CALCITE-4585]when run RelNode or Queryable,Confuse log info be thrown(xiong duan)
Posted by GitBox <gi...@apache.org>.
NobiGo commented on a change in pull request #2402:
URL: https://github.com/apache/calcite/pull/2402#discussion_r619986247
##########
File path: core/src/test/java/org/apache/calcite/test/ExceptionMessageTest.java
##########
@@ -141,4 +167,29 @@ private void runQuery(String sql) throws SQLException {
containsString("Object 'nonexistentTable' not found"));
}
}
+
+ @Test void testValidRelNodeQuery() throws SQLException {
+ final RelNode relNode = builder
+ .scan("test", "entries")
+ .project(builder.field("name"))
+ .build();
+ runQuery(relNode);
+ }
+
+ @Test void testRelNodeQueryException() throws SQLException {
+ try {
+ final RelNode relNode = builder
+ .scan("test", "entries")
+ .project(builder.call(SqlStdOperatorTable.ABS, builder.field("name")))
+ .build();
+ runQuery(relNode);
+ fail("Query badEntries should result in an exception");
+ } catch (RuntimeException e) {
+ assertThat(e.getMessage(),
+ equalTo("java.sql.SQLException: Error while preparing statement [\n"
+ + "LogicalProject($f0=[ABS($1)])\n"
Review comment:
Ok. I will change this to plan. Thanks for all of your advice.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] zabetak commented on a change in pull request #2402: [CALCITE-4585]when run RelNode or Queryable,Confuse log info be thrown(xiong duan)
Posted by GitBox <gi...@apache.org>.
zabetak commented on a change in pull request #2402:
URL: https://github.com/apache/calcite/pull/2402#discussion_r619077198
##########
File path: core/src/test/java/org/apache/calcite/test/ExceptionMessageTest.java
##########
@@ -141,4 +167,29 @@ private void runQuery(String sql) throws SQLException {
containsString("Object 'nonexistentTable' not found"));
}
}
+
+ @Test void testValidRelNodeQuery() throws SQLException {
+ final RelNode relNode = builder
+ .scan("test", "entries")
+ .project(builder.field("name"))
+ .build();
+ runQuery(relNode);
+ }
+
+ @Test void testRelNodeQueryException() throws SQLException {
Review comment:
There are test failures in CI (Windows). Please fix the problem with the line endings; check other tests cases comparing plans.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] NobiGo commented on a change in pull request #2402: [CALCITE-4585]when run RelNode or Queryable,Confuse log info be thrown(xiong duan)
Posted by GitBox <gi...@apache.org>.
NobiGo commented on a change in pull request #2402:
URL: https://github.com/apache/calcite/pull/2402#discussion_r619189915
##########
File path: core/src/test/java/org/apache/calcite/test/ExceptionMessageTest.java
##########
@@ -141,4 +167,29 @@ private void runQuery(String sql) throws SQLException {
containsString("Object 'nonexistentTable' not found"));
}
}
+
+ @Test void testValidRelNodeQuery() throws SQLException {
+ final RelNode relNode = builder
+ .scan("test", "entries")
+ .project(builder.field("name"))
+ .build();
+ runQuery(relNode);
+ }
+
+ @Test void testRelNodeQueryException() throws SQLException {
Review comment:
Yes. I will fix this.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] julianhyde commented on pull request #2402: [CALCITE-4585]when run RelNode error,Confuse log info be thrown(xiong duan)
Posted by GitBox <gi...@apache.org>.
julianhyde commented on pull request #2402:
URL: https://github.com/apache/calcite/pull/2402#issuecomment-827091969
@NobiGo, I refactored a little in https://github.com/julianhyde/calcite/tree/4585-relRunner-exception, and also fixed related bug https://issues.apache.org/jira/browse/CALCITE-4591. Let me know if there are any objections. I will squash & merge shortly.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] NobiGo commented on pull request #2402: [CALCITE-4585]when run RelNode error,Confuse log info be thrown(xiong duan)
Posted by GitBox <gi...@apache.org>.
NobiGo commented on pull request #2402:
URL: https://github.com/apache/calcite/pull/2402#issuecomment-827232976
@julianhyde [CALCITE-4591](https://issues.apache.org/jira/browse/CALCITE-4591) makes this issue more complete. I'm +1 for this
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] Aaaaaaron commented on a change in pull request #2402: [CALCITE-4585]when run RelNode or Queryable,Confuse log info be thrown(xiong duan)
Posted by GitBox <gi...@apache.org>.
Aaaaaaron commented on a change in pull request #2402:
URL: https://github.com/apache/calcite/pull/2402#discussion_r619939520
##########
File path: core/src/test/java/org/apache/calcite/test/ExceptionMessageTest.java
##########
@@ -141,4 +167,29 @@ private void runQuery(String sql) throws SQLException {
containsString("Object 'nonexistentTable' not found"));
}
}
+
+ @Test void testValidRelNodeQuery() throws SQLException {
+ final RelNode relNode = builder
+ .scan("test", "entries")
+ .project(builder.field("name"))
+ .build();
+ runQuery(relNode);
+ }
+
+ @Test void testRelNodeQueryException() throws SQLException {
+ try {
+ final RelNode relNode = builder
+ .scan("test", "entries")
+ .project(builder.call(SqlStdOperatorTable.ABS, builder.field("name")))
+ .build();
+ runQuery(relNode);
+ fail("Query badEntries should result in an exception");
+ } catch (RuntimeException e) {
+ assertThat(e.getMessage(),
+ equalTo("java.sql.SQLException: Error while preparing statement [\n"
+ + "LogicalProject($f0=[ABS($1)])\n"
Review comment:
Actually, I'm +1 for Danny, maybe change "statement" to "plan" is better. But the code before is also "Error while preparing statement"
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] NobiGo commented on pull request #2402: [CALCITE-4585]when run RelNode error,Confuse log info be thrown(xiong duan)
Posted by GitBox <gi...@apache.org>.
NobiGo commented on pull request #2402:
URL: https://github.com/apache/calcite/pull/2402#issuecomment-826790252
Hi, @julianhyde I have try to modify this. Thanks for your advice. This really helped me a lot
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] NobiGo commented on a change in pull request #2402: [CALCITE-4585]when run RelNode or Queryable,Confuse log info be thrown(xiong duan)
Posted by GitBox <gi...@apache.org>.
NobiGo commented on a change in pull request #2402:
URL: https://github.com/apache/calcite/pull/2402#discussion_r619004390
##########
File path: core/src/test/java/org/apache/calcite/test/ExceptionMessageTest.java
##########
@@ -141,4 +167,29 @@ private void runQuery(String sql) throws SQLException {
containsString("Object 'nonexistentTable' not found"));
}
}
+
+ @Test void testValidRelNodeQuery() throws SQLException {
+ final RelNode relNode = builder
+ .scan("test", "entries")
+ .project(builder.field("name"))
+ .build();
+ runQuery(relNode);
+ }
+
+ @Test void testRelNodeQueryException() throws SQLException {
+ try {
+ final RelNode relNode = builder
+ .scan("test", "entries")
+ .project(builder.call(SqlStdOperatorTable.ABS, builder.field("name")))
+ .build();
+ runQuery(relNode);
+ fail("Query badEntries should result in an exception");
+ } catch (RuntimeException e) {
+ assertThat(e.getMessage(),
+ equalTo("java.sql.SQLException: Error while preparing statement [\n"
+ + "LogicalProject($f0=[ABS($1)])\n"
Review comment:
Statement in Calcite may be include three types: Sql. RelNode . Queryable. This method Called prepareStatement_. My be we listen to others opinion.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] julianhyde closed pull request #2402: [CALCITE-4585]when run RelNode error,Confuse log info be thrown(xiong duan)
Posted by GitBox <gi...@apache.org>.
julianhyde closed pull request #2402:
URL: https://github.com/apache/calcite/pull/2402
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org