You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by "kramerul (via GitHub)" <gi...@apache.org> on 2024/02/16 16:15:47 UTC
[PR] [CALCITE-6268] Support implementing custom JdbcSchema [calcite]
kramerul opened a new pull request, #3692:
URL: https://github.com/apache/calcite/pull/3692
(no comment)
--
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: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6268] Support implementing custom JdbcSchema [calcite]
Posted by "kramerul (via GitHub)" <gi...@apache.org>.
kramerul commented on PR #3692:
URL: https://github.com/apache/calcite/pull/3692#issuecomment-1952056421
Hi @mihaibudiu,
I added a unit test for this feature.
--
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: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6268] Support implementing custom JdbcSchema [calcite]
Posted by "kramerul (via GitHub)" <gi...@apache.org>.
kramerul commented on PR #3692:
URL: https://github.com/apache/calcite/pull/3692#issuecomment-1953649441
> BTW: until the reviews are completed, it is recommended to just add new commits, so reviewers can easily see the new changes. Then you can squash the commits into a single one.
Thank you for the hint.
--
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: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6268] Support implementing custom JdbcSchema [calcite]
Posted by "mihaibudiu (via GitHub)" <gi...@apache.org>.
mihaibudiu merged PR #3692:
URL: https://github.com/apache/calcite/pull/3692
--
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: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6268] Support implementing custom JdbcSchema [calcite]
Posted by "mihaibudiu (via GitHub)" <gi...@apache.org>.
mihaibudiu commented on PR #3692:
URL: https://github.com/apache/calcite/pull/3692#issuecomment-1954847431
Now you can squash the commits into a single one in preparation for merging.
This looks fine for me, but if someone who knows more about the many ways JdbcSchema can be used wants to review, even better.
--
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: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6268] Support implementing custom JdbcSchema [calcite]
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #3692:
URL: https://github.com/apache/calcite/pull/3692#issuecomment-1956004806
## [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3692) **Quality Gate passed**
Issues
 [0 New issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3692&resolved=false&inNewCodePeriod=true)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3692&resolved=false&inNewCodePeriod=true)
 [59.1% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3692&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3692&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3692)
--
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: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6268] Support implementing custom JdbcSchema [calcite]
Posted by "kramerul (via GitHub)" <gi...@apache.org>.
kramerul commented on code in PR #3692:
URL: https://github.com/apache/calcite/pull/3692#discussion_r1495381585
##########
core/src/test/java/org/apache/calcite/test/JdbcTest.java:
##########
@@ -241,6 +245,55 @@ public class JdbcTest {
+ "(8,1,4))\n"
+ " as t(rn,val,expected)";
+ public static final String FOODMART_SCOTT_CUSTOM_MODEL = "{\n"
+ + " version: '1.0',\n"
+ + " schemas: [\n"
+ + " {\n"
+ + " type: 'custom',\n"
+ + " factory: '"
+ + JdbcCustomSchemaFactory.class.getName()
+ + "',\n"
+ + " name: 'SCOTT'\n"
+ + " }\n"
+ + " ]\n"
+ + "}";
+
+
+ /**
+ * Tests class fro custom JDBC schema.
Review Comment:
I don't know where I could add this comment.
Today, it's not possible to implement a custom {{JdcbSchema}} because the generated code always calls {{CalciteSchema::unwrap}}. And there is no other way to get a {{DataSource}} from a {{JdbcSchema}}.
We have problems using {{JdbcSchema}} because we are using a database with many 1000 schemas each containing up to 500000 tables. During validation, this will require up to 30 seconds to read all those tables into memory {{JdbcSchema::computeTables}}. Additionally, this requires a lot of memory to hold 500000 tables inside {{JdcbSchema}}. Using {{JdbcSchema::snapshot}} is also not an option because there is no way to invalidate the cache.
Therefore, we would like to implement our own special way to maintain the required tables in memory.
--
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: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6268] Support implementing custom JdbcSchema [calcite]
Posted by "kramerul (via GitHub)" <gi...@apache.org>.
kramerul commented on PR #3692:
URL: https://github.com/apache/calcite/pull/3692#issuecomment-1955938337
I squashed the commits now.
--
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: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6268] Support implementing custom JdbcSchema [calcite]
Posted by "mihaibudiu (via GitHub)" <gi...@apache.org>.
mihaibudiu commented on code in PR #3692:
URL: https://github.com/apache/calcite/pull/3692#discussion_r1494969469
##########
core/src/test/java/org/apache/calcite/test/JdbcTest.java:
##########
@@ -241,6 +245,55 @@ public class JdbcTest {
+ "(8,1,4))\n"
+ " as t(rn,val,expected)";
+ public static final String FOODMART_SCOTT_CUSTOM_MODEL = "{\n"
+ + " version: '1.0',\n"
+ + " schemas: [\n"
+ + " {\n"
+ + " type: 'custom',\n"
+ + " factory: '"
+ + JdbcCustomSchemaFactory.class.getName()
+ + "',\n"
+ + " name: 'SCOTT'\n"
+ + " }\n"
+ + " ]\n"
+ + "}";
+
+
+ /**
+ * Tests class fro custom JDBC schema.
Review Comment:
typo
##########
core/src/test/java/org/apache/calcite/test/JdbcTest.java:
##########
@@ -241,6 +245,55 @@ public class JdbcTest {
+ "(8,1,4))\n"
+ " as t(rn,val,expected)";
+ public static final String FOODMART_SCOTT_CUSTOM_MODEL = "{\n"
+ + " version: '1.0',\n"
+ + " schemas: [\n"
+ + " {\n"
+ + " type: 'custom',\n"
+ + " factory: '"
+ + JdbcCustomSchemaFactory.class.getName()
+ + "',\n"
+ + " name: 'SCOTT'\n"
+ + " }\n"
+ + " ]\n"
+ + "}";
+
+
+ /**
+ * Tests class fro custom JDBC schema.
+ */
+ public static class JdbcCustomSchema extends DelegatingSchema implements Wrapper {
+
+ public JdbcCustomSchema(SchemaPlus parentSchema, String name) {
+ super(JdbcSchema.create(parentSchema, name, getDataSource(), SCOTT.catalog, SCOTT.schema));
+ }
+
+ private static DataSource getDataSource() {
+ return JdbcSchema.dataSource(SCOTT.url, SCOTT.driver, SCOTT.username, SCOTT.password);
+ }
+
+ @Override public Schema snapshot(SchemaVersion version) {
+ return this;
+ }
+
+ @Override public <T extends Object> @Nullable T unwrap(Class<T> clazz) {
+ if (schema instanceof Wrapper) {
+ return ((Wrapper) schema).unwrap(clazz);
+ }
+ return null;
+ }
+ }
+
+ /**
+ * Tests class fro custom JDBC schema factory.
Review Comment:
same typo.
##########
core/src/test/java/org/apache/calcite/test/JdbcTest.java:
##########
@@ -241,6 +245,55 @@ public class JdbcTest {
+ "(8,1,4))\n"
+ " as t(rn,val,expected)";
+ public static final String FOODMART_SCOTT_CUSTOM_MODEL = "{\n"
+ + " version: '1.0',\n"
+ + " schemas: [\n"
+ + " {\n"
+ + " type: 'custom',\n"
+ + " factory: '"
+ + JdbcCustomSchemaFactory.class.getName()
+ + "',\n"
+ + " name: 'SCOTT'\n"
+ + " }\n"
+ + " ]\n"
+ + "}";
+
+
+ /**
+ * Tests class fro custom JDBC schema.
Review Comment:
The question I still have: how are users supposed to use the new capabilities you are enabling?
Can you add a longer comment giving a recipe for the problem you are solving and how people are supposed to solve it using these tools - pointing them to this example.
--
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: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6268] Support implementing custom JdbcSchema [calcite]
Posted by "kramerul (via GitHub)" <gi...@apache.org>.
kramerul commented on code in PR #3692:
URL: https://github.com/apache/calcite/pull/3692#discussion_r1495381585
##########
core/src/test/java/org/apache/calcite/test/JdbcTest.java:
##########
@@ -241,6 +245,55 @@ public class JdbcTest {
+ "(8,1,4))\n"
+ " as t(rn,val,expected)";
+ public static final String FOODMART_SCOTT_CUSTOM_MODEL = "{\n"
+ + " version: '1.0',\n"
+ + " schemas: [\n"
+ + " {\n"
+ + " type: 'custom',\n"
+ + " factory: '"
+ + JdbcCustomSchemaFactory.class.getName()
+ + "',\n"
+ + " name: 'SCOTT'\n"
+ + " }\n"
+ + " ]\n"
+ + "}";
+
+
+ /**
+ * Tests class fro custom JDBC schema.
Review Comment:
I don't know where I could add this comment.
Today, it's not possible to implement a custom `JdcbSchema` because the generated code always calls `CalciteSchema::unwrap`. And there is no other way to get a `DataSource` from a `JdbcSchema`.
We have problems using `JdbcSchema` because we are using a database with many 1000 schemas each containing up to 500000 tables. During validation, this will require up to 30 seconds to read all those tables into memory `JdbcSchema::computeTables`. Additionally, this requires a lot of memory to hold 500000 tables inside `JdcbSchema`. Using `JdbcSchema::snapshot` is also not an option because there is no way to invalidate the cache.
Therefore, we would like to implement our own `JdbcSchema` to maintain the required tables in memory.
--
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: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6268] Support implementing custom JdbcSchema [calcite]
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #3692:
URL: https://github.com/apache/calcite/pull/3692#issuecomment-1951929853
## [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3692) **Quality Gate passed**
Issues
 [0 New issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3692&resolved=false&inNewCodePeriod=true)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3692&resolved=false&inNewCodePeriod=true)
 [59.1% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3692&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3692&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3692)
--
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: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6268] Support implementing custom JdbcSchema [calcite]
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #3692:
URL: https://github.com/apache/calcite/pull/3692#issuecomment-1948866156
## [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3692) **Quality Gate passed**
Issues
 [0 New issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3692&resolved=false&inNewCodePeriod=true)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3692&resolved=false&inNewCodePeriod=true)
 [59.1% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3692&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3692&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3692)
--
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: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6268] Support implementing custom JdbcSchema [calcite]
Posted by "kramerul (via GitHub)" <gi...@apache.org>.
kramerul commented on code in PR #3692:
URL: https://github.com/apache/calcite/pull/3692#discussion_r1495381585
##########
core/src/test/java/org/apache/calcite/test/JdbcTest.java:
##########
@@ -241,6 +245,55 @@ public class JdbcTest {
+ "(8,1,4))\n"
+ " as t(rn,val,expected)";
+ public static final String FOODMART_SCOTT_CUSTOM_MODEL = "{\n"
+ + " version: '1.0',\n"
+ + " schemas: [\n"
+ + " {\n"
+ + " type: 'custom',\n"
+ + " factory: '"
+ + JdbcCustomSchemaFactory.class.getName()
+ + "',\n"
+ + " name: 'SCOTT'\n"
+ + " }\n"
+ + " ]\n"
+ + "}";
+
+
+ /**
+ * Tests class fro custom JDBC schema.
Review Comment:
I don't know where I could add this comment.
Today, it's not possible to implement a custom `JdcbSchema` because the generated code always calls `CalciteSchema::unwrap`, which does en explicit check for `JdcbSchema`. And there is no other way to get a `DataSource` from a `JdbcSchema`.
We have problems using `JdbcSchema` because we are using a database with many 1000 schemas each containing up to 500000 tables. During validation, this will require up to 30 seconds to read all those tables into memory `JdbcSchema::computeTables`. Additionally, this requires a lot of memory to hold 500000 tables inside `JdcbSchema`. Using `JdbcSchema::snapshot` is also not an option because there is no way to invalidate the cache.
Therefore, we would like to implement our own `JdbcSchema` to maintain the required tables in memory.
--
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: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6268] Support implementing custom JdbcSchema [calcite]
Posted by "kramerul (via GitHub)" <gi...@apache.org>.
kramerul commented on code in PR #3692:
URL: https://github.com/apache/calcite/pull/3692#discussion_r1495381585
##########
core/src/test/java/org/apache/calcite/test/JdbcTest.java:
##########
@@ -241,6 +245,55 @@ public class JdbcTest {
+ "(8,1,4))\n"
+ " as t(rn,val,expected)";
+ public static final String FOODMART_SCOTT_CUSTOM_MODEL = "{\n"
+ + " version: '1.0',\n"
+ + " schemas: [\n"
+ + " {\n"
+ + " type: 'custom',\n"
+ + " factory: '"
+ + JdbcCustomSchemaFactory.class.getName()
+ + "',\n"
+ + " name: 'SCOTT'\n"
+ + " }\n"
+ + " ]\n"
+ + "}";
+
+
+ /**
+ * Tests class fro custom JDBC schema.
Review Comment:
I don't know where I could add this comment.
Today, it's not possible to implement a custom `JdcbSchema` because the generated code always calls `CalciteSchema::unwrap`. And there is no other way to get a `DataSource` from a `JdbcSchema`.
We have problems using `JdbcSchema` because we are using a database with many 1000 schemas each containing up to 500000 tables. During validation, this will require up to 30 seconds to read all those tables into memory `JdbcSchema::computeTables`. Additionally, this requires a lot of memory to hold 500000 tables inside `JdcbSchema`. Using `JdbcSchema::snapshot` is also not an option because there is no way to invalidate the cache.
Therefore, we would like to implement our own special way to maintain the required tables in memory.
--
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: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6268] Support implementing custom JdbcSchema [calcite]
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #3692:
URL: https://github.com/apache/calcite/pull/3692#issuecomment-1953699422
## [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3692) **Quality Gate passed**
Issues
 [0 New issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3692&resolved=false&inNewCodePeriod=true)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3692&resolved=false&inNewCodePeriod=true)
 [59.1% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3692&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3692&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3692)
--
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: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6268] Support implementing custom JdbcSchema [calcite]
Posted by "mihaibudiu (via GitHub)" <gi...@apache.org>.
mihaibudiu commented on PR #3692:
URL: https://github.com/apache/calcite/pull/3692#issuecomment-1953163472
BTW: until the reviews are completed, it is recommended to just add new commits, so reviewers can easily see the new changes. Then you can squash the commits into a single one.
--
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: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org