Re: [PR] [CALCITE-6249] RelNode::estimatedRowCount should not be used in computeSelfCost [calcite]

2024-02-13 Thread via GitHub
kramerul commented on PR #3674: URL: https://github.com/apache/calcite/pull/3674#issuecomment-1943144254 > The title says something should not be used, but there is no test to tell me that the claim of the JIRA case is true or has been fixed by this PR. In the current code base,

Re: [PR] [CALCITE-6239] Add a postgis dialect that supports ST functions [calcite]

2024-02-13 Thread via GitHub
sonarcloud[bot] commented on PR #3668: URL: https://github.com/apache/calcite/pull/3668#issuecomment-1942774265 ## [![Quality Gate Passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/qg-passed-20px.png 'Quality Gate

Re: [PR] [CALCITE-6239] Add a postgis dialect that supports ST functions [calcite]

2024-02-13 Thread via GitHub
julianhyde commented on code in PR #3668: URL: https://github.com/apache/calcite/pull/3668#discussion_r1488656193 ## core/src/main/java/org/apache/calcite/rel/type/RelDataTypeFactoryImpl.java: ## @@ -39,9 +39,11 @@ import com.google.common.collect.Interners; import

Re: [PR] [CALCITE-6248] Illegal dates are accepted by casts [calcite]

2024-02-13 Thread via GitHub
mihaibudiu merged PR #3685: URL: https://github.com/apache/calcite/pull/3685 -- 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:

(calcite) branch main updated (e05b0473a0 -> c774c313a8)

2024-02-13 Thread mbudiu
This is an automated email from the ASF dual-hosted git repository. mbudiu pushed a change to branch main in repository https://gitbox.apache.org/repos/asf/calcite.git from e05b0473a0 [CALCITE-6250] Limitations of MongoDB adapter are not documented add c774c313a8 [CALCITE-6248]

Re: [PR] [CALCITE-6239] Add a postgis dialect that supports ST functions [calcite]

2024-02-13 Thread via GitHub
bchapuis commented on code in PR #3668: URL: https://github.com/apache/calcite/pull/3668#discussion_r1488560237 ## core/src/test/java/org/apache/calcite/sql/test/SqlTypeNameTest.java: ## @@ -172,7 +173,7 @@ class SqlTypeNameTest { @Test void testOther() { SqlTypeName tn

Re: [PR] [CALCITE-6239] Add a postgis dialect that supports ST functions [calcite]

2024-02-13 Thread via GitHub
bchapuis commented on code in PR #3668: URL: https://github.com/apache/calcite/pull/3668#discussion_r1488530763 ## core/src/main/java/org/apache/calcite/sql/SqlKind.java: ## @@ -1140,27 +1140,447 @@ public enum SqlKind { // Spatial functions. They are registered as

Re: [PR] [CALCITE-6210] Cast to VARBINARY causes an assertion failure [calcite]

2024-02-13 Thread via GitHub
julianhyde commented on code in PR #3635: URL: https://github.com/apache/calcite/pull/3635#discussion_r1488264229 ## core/src/main/java/org/apache/calcite/rex/RexUtil.java: ## @@ -1683,7 +1683,7 @@ public static boolean isLosslessCast(RexNode node) { * * @param source

Re: [PR] [CALCITE-6249] RelNode::estimatedRowCount should not be used in computeSelfCost [calcite]

2024-02-13 Thread via GitHub
tanclary commented on PR #3674: URL: https://github.com/apache/calcite/pull/3674#issuecomment-1942006724 > Thanks @rubenada for your detailed explanation. For me, this PR is more like a refactoring. Therefore, I would also prefer not writing a unit test. The existing unit tests ensure that

Re: [PR] [CALCITE-6239] Add a postgis dialect that supports ST functions [calcite]

2024-02-13 Thread via GitHub
bchapuis commented on code in PR #3668: URL: https://github.com/apache/calcite/pull/3668#discussion_r1487873628 ## core/src/main/java/org/apache/calcite/rel/type/RelDataTypeFactoryImpl.java: ## @@ -39,9 +39,11 @@ import com.google.common.collect.Interners; import

Re: [PR] [CALCITE-6239] Add a postgis dialect that supports ST functions [calcite]

2024-02-13 Thread via GitHub
bchapuis commented on code in PR #3668: URL: https://github.com/apache/calcite/pull/3668#discussion_r1487852604 ## core/src/main/java/org/apache/calcite/rel/type/RelDataTypeFactoryImpl.java: ## @@ -39,9 +39,11 @@ import com.google.common.collect.Interners; import

Re: [PR] [CALCITE-6239] Add a postgis dialect that supports ST functions [calcite]

2024-02-13 Thread via GitHub
bchapuis commented on code in PR #3668: URL: https://github.com/apache/calcite/pull/3668#discussion_r1487860243 ## core/src/main/java/org/apache/calcite/adapter/enumerable/RexToLixTranslator.java: ## @@ -312,7 +312,9 @@ private Expression getConvertExpression( case CHAR:

Re: [PR] [CALCITE-6239] Add a postgis dialect that supports ST functions [calcite]

2024-02-13 Thread via GitHub
bchapuis commented on code in PR #3668: URL: https://github.com/apache/calcite/pull/3668#discussion_r1487856703 ## core/src/main/java/org/apache/calcite/sql/dialect/PostgisGeometryDecoder.java: ## @@ -0,0 +1,191 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under

Re: [PR] [CALCITE-6239] Add a postgis dialect that supports ST functions [calcite]

2024-02-13 Thread via GitHub
bchapuis commented on code in PR #3668: URL: https://github.com/apache/calcite/pull/3668#discussion_r1487852604 ## core/src/main/java/org/apache/calcite/rel/type/RelDataTypeFactoryImpl.java: ## @@ -39,9 +39,11 @@ import com.google.common.collect.Interners; import

Re: [PR] [CALCITE-6239] Add a postgis dialect that supports ST functions [calcite]

2024-02-13 Thread via GitHub
bchapuis commented on code in PR #3668: URL: https://github.com/apache/calcite/pull/3668#discussion_r1487541098 ## core/src/main/java/org/apache/calcite/adapter/jdbc/JdbcSchema.java: ## @@ -433,6 +433,12 @@ private static RelDataType sqlType(RelDataTypeFactory typeFactory, int

Re: [PR] [CALCITE-6249] RelNode::estimatedRowCount should not be used in computeSelfCost [calcite]

2024-02-13 Thread via GitHub
kramerul commented on PR #3674: URL: https://github.com/apache/calcite/pull/3674#issuecomment-1941039599 Thanks @rubenada for you detailed explanation. For me, this PR is more like a refactoring. Therefore, I would also prefer not writing a unit test. The existing unit tests ensure that

Re: [PR] [CALCITE-6210] Cast to VARBINARY causes an assertion failure [calcite]

2024-02-13 Thread via GitHub
gortiz commented on PR #3635: URL: https://github.com/apache/calcite/pull/3635#issuecomment-1940713121 I've tried, but for some reason I my credentials in Apache Jira are not valid anymore. Maybe something got messed up when I became Pinot Contributor or maybe I'm just unable to remember