Re: [PR] [CALCITE-6115] Interval type specifier with zero fractional second precision does not pass validation [calcite]
asolimando merged PR #3563: URL: https://github.com/apache/calcite/pull/3563 -- 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-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [CALCITE-6115] Interval type specifier with zero fractional second precision does not pass validation [calcite]
LeonidChistov commented on PR #3563: URL: https://github.com/apache/calcite/pull/3563#issuecomment-1856031725 @asolimando thanks a lot for explanation. Could someone help now with merging the PR? -- 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-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [CALCITE-6115] Interval type specifier with zero fractional second precision does not pass validation [calcite]
asolimando commented on PR #3563:
URL: https://github.com/apache/calcite/pull/3563#issuecomment-1855629734
> @libenchao Thanks. Do I understand correctly that this test run is not
required (in general) to merge a PR?
They are not, but they will be run when the PR is merged, for changes to
core functions that can have a broad impact I'd say it's recommended, in order
to catch issues early on without waiting for the `main` branch to break.
What is a "core function" is a fuzzy definition though, you can look for the
`@Tag("slow")` tag to get a sense of that is covered by slow tests (even
though, the scope is pretty broad).
If anyone has some more precise recommendations on this topic, I guess it
would be a great addition to the developer's guide on the website.
--
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-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] [CALCITE-6115] Interval type specifier with zero fractional second precision does not pass validation [calcite]
LeonidChistov commented on PR #3563: URL: https://github.com/apache/calcite/pull/3563#issuecomment-1855588082 @libenchao Thanks. Do I understand correctly that this test run is not required (in general) to merge a PR? -- 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-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [CALCITE-6115] Interval type specifier with zero fractional second precision does not pass validation [calcite]
libenchao commented on PR #3563: URL: https://github.com/apache/calcite/pull/3563#issuecomment-1854913057 @LeonidChistov @mihaibudiu The slow tests are trigger by adding a label "slow-tests-needed", see here: https://github.com/apache/calcite/blob/8c640da341efa2bc9a711290a4eca978a6174574/.github/workflows/main.yml#L401-L404 -- 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-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [CALCITE-6115] Interval type specifier with zero fractional second precision does not pass validation [calcite]
mihaibudiu commented on PR #3563: URL: https://github.com/apache/calcite/pull/3563#issuecomment-1854547993 I am not aware of the Calcite CI running slow tests. I don't know how to do 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: commits-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [CALCITE-6115] Interval type specifier with zero fractional second precision does not pass validation [calcite]
LeonidChistov commented on PR #3563: URL: https://github.com/apache/calcite/pull/3563#issuecomment-1854544376 @mihaibudiu Thanks a lot for review. BTW, do you have a power to trigger `Slow Tests` run? Looks like it doesn't run by default. -- 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-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [CALCITE-6115] Interval type specifier with zero fractional second precision does not pass validation [calcite]
sonarcloud[bot] commented on PR #3563: URL: https://github.com/apache/calcite/pull/3563#issuecomment-1853612371 ## [](https://sonarcloud.io/dashboard?id=apache_calcite=3563) **Quality Gate passed** The SonarCloud Quality Gate passed, but some issues were introduced. [1 New issue](https://sonarcloud.io/project/issues?id=apache_calcite=3563=false=true) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3563=false=true) No data about Coverage [80.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3563=new_duplicated_lines_density=list) [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite=3563) -- 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-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [CALCITE-6115] Interval type specifier with zero fractional second precision does not pass validation [calcite]
LeonidChistov commented on PR #3563: URL: https://github.com/apache/calcite/pull/3563#issuecomment-1853588606 @mihaibudiu What exactly do you mean regarding SQL dialects support? Since Calcite is a framework for building RDBMS query optimizers, I think it make sense for it to support this feature described in SQL standard, even if some databases would not support it. It is always possible to add custom checks that would prohibit zero fractional precision, but if it is prohibited on framework level, there is no easy way to disable these checks if you try to build SQL-standard compliant RDBMS. -- 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-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [CALCITE-6115] Interval type specifier with zero fractional second precision does not pass validation [calcite]
LeonidChistov commented on code in PR #3563:
URL: https://github.com/apache/calcite/pull/3563#discussion_r1425096618
##
core/src/test/java/org/apache/calcite/test/SqlToRelConverterTest.java:
##
@@ -143,6 +143,10 @@ public static void checkActualAndReferenceFiles() {
sql("select interval mgr hour as h from emp").ok();
}
+ @Test void testIntervalSecondNoFractionalPart() {
Review Comment:
Thanks, fixed.
--
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-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] [CALCITE-6115] Interval type specifier with zero fractional second precision does not pass validation [calcite]
mihaibudiu commented on PR #3563: URL: https://github.com/apache/calcite/pull/3563#issuecomment-1846410408 I am not a SQL expert, but if other dialects accept this, I think it's good. -- 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-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [CALCITE-6115] Interval type specifier with zero fractional second precision does not pass validation [calcite]
mihaibudiu commented on code in PR #3563:
URL: https://github.com/apache/calcite/pull/3563#discussion_r1419830987
##
core/src/test/java/org/apache/calcite/test/SqlToRelConverterTest.java:
##
@@ -143,6 +143,10 @@ public static void checkActualAndReferenceFiles() {
sql("select interval mgr hour as h from emp").ok();
}
+ @Test void testIntervalSecondNoFractionalPart() {
Review Comment:
it is customary to add a comment with the JIRA issue that is being solved.
Please follow the other examples in this file that start with "Test case
for".
Note that everything is important, including the last period.
--
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-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] [CALCITE-6115] Interval type specifier with zero fractional second precision does not pass validation [calcite]
sonarcloud[bot] commented on PR #3563: URL: https://github.com/apache/calcite/pull/3563#issuecomment-1845114445 Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_calcite=3563) [](https://sonarcloud.io/project/issues?id=apache_calcite=3563=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite=3563=false=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite=3563=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite=3563=false=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_calcite=3563=false=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite=3563=false=VULNERABILITY) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3563=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3563=false=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3563=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/issues?id=apache_calcite=3563=false=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_calcite=3563=false=CODE_SMELL) [1 Code Smell](https://sonarcloud.io/project/issues?id=apache_calcite=3563=false=CODE_SMELL) [](https://sonarcloud.io/component_measures?id=apache_calcite=3563=coverage=list) No Coverage information [](https://sonarcloud.io/component_measures?id=apache_calcite=3563=new_duplicated_lines_density=list) [80.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite=3563=new_duplicated_lines_density=list) -- 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-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
