You are viewing a plain text version of this content. The canonical link for it is here.
Posted to gitbox@hive.apache.org by "kasakrisz (via GitHub)" <gi...@apache.org> on 2023/05/25 11:49:45 UTC
[GitHub] [hive] kasakrisz opened a new pull request, #4356: HIVE-27373: Unable to pushdown partition filter with unquoted date literal type to metastore
kasakrisz opened a new pull request, #4356:
URL: https://github.com/apache/hive/pull/4356
<!--
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?
Remove support of unquoted timestamp literals without type information in filter expressions when pulling partition metadata from HMS.
### Why are the changes needed?
The filter expression is transferred from `hs2` to `hms` in byte array format which is deserialized to `String`. Then this string is parsed via `ANTLR` to build an `ExpressionTree` in HMS. The date literal is a suffix of timestamp literal
https://github.com/apache/hive/blob/7bfc54ffc8baf5f3e7de326b750fe9355b11132b/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/parser/Filter.g#L530-L538
Timestamp literal also contains a space character which makes ANTLR unable to parse the expression.
Unfortunately the unquoted date representation without type name is passed by other legacy clients like older version of Spark so this patch makes the api backward compatible.
Not supporting unquoted timestamp literals is not problematic since timestamp support was only recently added.
### Does this PR introduce _any_ user-facing change?
No.
### How was this patch tested?
```
mvn test -Dtest=TestPartFilterExprUtil -pl standalone-metastore/metastore-server -Drat.skip
```
--
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 diff in pull request #4356: HIVE-27373: Unable to pushdown partition filter with unquoted date literal type to metastore
Posted by "kasakrisz (via GitHub)" <gi...@apache.org>.
kasakrisz commented on code in PR #4356:
URL: https://github.com/apache/hive/pull/4356#discussion_r1211740858
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/parser/Filter.g:
##########
@@ -549,7 +549,6 @@ DateLiteral
TimestampLiteral
:
KW_TIMESTAMP '\'' TimestampString '\'' { extractTimestamp(getText()) != null }?
- | TimestampString { extractTimestamp(getText()) != null }?
Review Comment:
It is possible with a slight change in you proposed rule.
Since the unquoted parts conflicts I had to move the unquoted ts branch to always execute predicate.
--
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] sonarcloud[bot] commented on pull request #4356: HIVE-27373: Unable to pushdown partition filter with unquoted date literal type to metastore
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #4356:
URL: https://github.com/apache/hive/pull/4356#issuecomment-1574112550
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=4356)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4356&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4356&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4356&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4356&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4356&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4356&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4356&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4356&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4356&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4356&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4356&resolved=false&types=CODE_SMELL) [1 Code Smell](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4356&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=4356&metric=coverage&view=list) No Coverage information
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=4356&metric=duplicated_lines_density&view=list) No Duplication information
--
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 diff in pull request #4356: HIVE-27373: Unable to pushdown partition filter with unquoted date literal type to metastore
Posted by "kasakrisz (via GitHub)" <gi...@apache.org>.
kasakrisz commented on code in PR #4356:
URL: https://github.com/apache/hive/pull/4356#discussion_r1211743343
##########
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestPartFilterExprUtil.java:
##########
@@ -0,0 +1,74 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.hive.metastore;
+
+import org.apache.hadoop.hive.metastore.annotation.MetastoreUnitTest;
+import org.apache.hadoop.hive.metastore.api.MetaException;
+import org.apache.hadoop.hive.metastore.parser.ExpressionTree;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import static org.hamcrest.core.Is.is;
+import static org.hamcrest.MatcherAssert.assertThat;
+
+@Category(MetastoreUnitTest.class)
+public class TestPartFilterExprUtil {
Review Comment:
Added tests but these are not parameterized. I prefer write the exact expression to parse into each test case instead of generate it because it is easier to understand what the test does.
--
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 diff in pull request #4356: HIVE-27373: Unable to pushdown partition filter with unquoted date literal type to metastore
Posted by "kasakrisz (via GitHub)" <gi...@apache.org>.
kasakrisz commented on code in PR #4356:
URL: https://github.com/apache/hive/pull/4356#discussion_r1211743343
##########
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestPartFilterExprUtil.java:
##########
@@ -0,0 +1,74 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.hive.metastore;
+
+import org.apache.hadoop.hive.metastore.annotation.MetastoreUnitTest;
+import org.apache.hadoop.hive.metastore.api.MetaException;
+import org.apache.hadoop.hive.metastore.parser.ExpressionTree;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import static org.hamcrest.core.Is.is;
+import static org.hamcrest.MatcherAssert.assertThat;
+
+@Category(MetastoreUnitTest.class)
+public class TestPartFilterExprUtil {
Review Comment:
Added tests
--
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 diff in pull request #4356: HIVE-27373: Unable to pushdown partition filter with unquoted date literal type to metastore
Posted by "kasakrisz (via GitHub)" <gi...@apache.org>.
kasakrisz commented on code in PR #4356:
URL: https://github.com/apache/hive/pull/4356#discussion_r1212780121
##########
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestPartFilterExprUtil.java:
##########
@@ -0,0 +1,74 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.hive.metastore;
+
+import org.apache.hadoop.hive.metastore.annotation.MetastoreUnitTest;
+import org.apache.hadoop.hive.metastore.api.MetaException;
+import org.apache.hadoop.hive.metastore.parser.ExpressionTree;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import static org.hamcrest.core.Is.is;
+import static org.hamcrest.MatcherAssert.assertThat;
+
+@Category(MetastoreUnitTest.class)
+public class TestPartFilterExprUtil {
+
+ @Test
+ public void testMakeExpressionTreeWhenDateLiteralTypeIsNotSpecified() throws MetaException {
+ ExpressionTree expressionTree = PartFilterExprUtil.makeExpressionTree("(j = 1990-11-10 or j = 1990-11-11 or j = 1990-11-12)");
+ assertThat(expressionTree.getRoot().toString(), is("TreeNode{lhs=TreeNode{lhs=LeafNode{keyName='j', operator='=', value=1990-11-10}, andOr='OR', rhs=LeafNode{keyName='j', operator='=', value=1990-11-11}}, andOr='OR', rhs=LeafNode{keyName='j', operator='=', value=1990-11-12}}"));
+ }
+
+ @Test
+ public void testMakeExpressionTreeWhenDateLiteralIsQuoted() throws MetaException {
+ ExpressionTree expressionTree = PartFilterExprUtil.makeExpressionTree("(j = '1990-11-10' or j = '1990-11-11' or j = '1990-11-12')");
Review Comment:
I was too eager to make the keywords optional so I reverted this. Keep treat quoted literals as date like string literals.
https://github.com/apache/hive/blob/a5d7f72dfe51fe8119d209bf61c64b8e94f2f42b/ql/src/test/queries/clientpositive/temp_table_partition_date.q#L53-L58
--
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 diff in pull request #4356: HIVE-27373: Unable to pushdown partition filter with unquoted date literal type to metastore
Posted by "kasakrisz (via GitHub)" <gi...@apache.org>.
kasakrisz commented on code in PR #4356:
URL: https://github.com/apache/hive/pull/4356#discussion_r1211740858
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/parser/Filter.g:
##########
@@ -549,7 +549,6 @@ DateLiteral
TimestampLiteral
:
KW_TIMESTAMP '\'' TimestampString '\'' { extractTimestamp(getText()) != null }?
- | TimestampString { extractTimestamp(getText()) != null }?
Review Comment:
It seems that is possible with a slight change in you proposed rule.
Since the unquoted parts conflicts I had to move the unquoted ts branch to always execute predicate.
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/PartFilterExprUtil.java:
##########
@@ -101,7 +102,8 @@ public static PartitionExpressionProxy createExpressionProxy(Configuration conf)
* @param filter Filter.
* @return Expression tree. Null if there was an error.
*/
- private static ExpressionTree makeExpressionTree(String filter) throws MetaException {
+ @VisibleForTesting
+ static ExpressionTree makeExpressionTree(String filter) throws MetaException {
Review Comment:
Done.
--
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] sonarcloud[bot] commented on pull request #4356: HIVE-27373: Unable to pushdown partition filter with unquoted date literal type to metastore
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #4356:
URL: https://github.com/apache/hive/pull/4356#issuecomment-1572132998
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=4356)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4356&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4356&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4356&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4356&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4356&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4356&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4356&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4356&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4356&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4356&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4356&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4356&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=4356&metric=coverage&view=list) No Coverage information
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=4356&metric=duplicated_lines_density&view=list) No Duplication information
--
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] sonarcloud[bot] commented on pull request #4356: HIVE-27373: Unable to pushdown partition filter with unquoted date literal type to metastore
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #4356:
URL: https://github.com/apache/hive/pull/4356#issuecomment-1568005104
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=4356)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4356&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4356&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4356&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4356&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4356&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4356&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4356&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4356&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4356&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4356&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4356&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4356&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=4356&metric=coverage&view=list) No Coverage information
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=4356&metric=duplicated_lines_density&view=list) No Duplication information
--
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 diff in pull request #4356: HIVE-27373: Unable to pushdown partition filter with unquoted date literal type to metastore
Posted by "kasakrisz (via GitHub)" <gi...@apache.org>.
kasakrisz commented on code in PR #4356:
URL: https://github.com/apache/hive/pull/4356#discussion_r1211742284
##########
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestPartFilterExprUtil.java:
##########
@@ -0,0 +1,74 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.hive.metastore;
+
+import org.apache.hadoop.hive.metastore.annotation.MetastoreUnitTest;
+import org.apache.hadoop.hive.metastore.api.MetaException;
+import org.apache.hadoop.hive.metastore.parser.ExpressionTree;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import static org.hamcrest.core.Is.is;
+import static org.hamcrest.MatcherAssert.assertThat;
+
+@Category(MetastoreUnitTest.class)
+public class TestPartFilterExprUtil {
+
+ @Test
+ public void testMakeExpressionTreeWhenDateLiteralTypeIsNotSpecified() throws MetaException {
+ ExpressionTree expressionTree = PartFilterExprUtil.makeExpressionTree("(j = 1990-11-10 or j = 1990-11-11 or j = 1990-11-12)");
+ assertThat(expressionTree.getRoot().toString(), is("TreeNode{lhs=TreeNode{lhs=LeafNode{keyName='j', operator='=', value=1990-11-10}, andOr='OR', rhs=LeafNode{keyName='j', operator='=', value=1990-11-11}}, andOr='OR', rhs=LeafNode{keyName='j', operator='=', value=1990-11-12}}"));
+ }
+
+ @Test
+ public void testMakeExpressionTreeWhenDateLiteralIsQuoted() throws MetaException {
+ ExpressionTree expressionTree = PartFilterExprUtil.makeExpressionTree("(j = '1990-11-10' or j = '1990-11-11' or j = '1990-11-12')");
+ assertThat(expressionTree.getRoot().toString(), is("TreeNode{lhs=TreeNode{lhs=LeafNode{keyName='j', operator='=', value=1990-11-10}, andOr='OR', rhs=LeafNode{keyName='j', operator='=', value=1990-11-11}}, andOr='OR', rhs=LeafNode{keyName='j', operator='=', value=1990-11-12}}"));
+ }
+
+ @Test
+ public void testMakeExpressionTreeWhenDateLiteralTypeIsSpecified() throws MetaException {
+ ExpressionTree expressionTree = PartFilterExprUtil.makeExpressionTree("(j) IN (DATE'1990-11-10', DATE'1990-11-11', DATE'1990-11-12')");
+ assertThat(expressionTree.getRoot().toString(), is("TreeNode{lhs=TreeNode{lhs=LeafNode{keyName='j', operator='=', value=1990-11-10}, andOr='OR', rhs=LeafNode{keyName='j', operator='=', value=1990-11-11}}, andOr='OR', rhs=LeafNode{keyName='j', operator='=', value=1990-11-12}}"));
+ }
+
+ @Test
+ public void testMakeExpressionTreeWhenTimestampLiteralTypeIsSpecified() throws MetaException {
+ ExpressionTree expressionTree = PartFilterExprUtil.makeExpressionTree("(j) IN (TIMESTAMP'2000-01-01 01:00:00', TIMESTAMP'2000-01-01 01:42:00')");
+ assertThat(expressionTree.getRoot().toString(), is("TreeNode{lhs=LeafNode{keyName='j', operator='=', value=2000-01-01 01:00:00.0}, andOr='OR', rhs=LeafNode{keyName='j', operator='=', value=2000-01-01 01:42:00.0}}"));
+ }
+
+ @Test
+ public void testMakeExpressionTreeWhenTimestampLiteralIsQuoted() throws MetaException {
Review Comment:
I managed to keep the support.
--
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] sonarcloud[bot] commented on pull request #4356: HIVE-27373: Unable to pushdown partition filter with unquoted date literal type to metastore
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #4356:
URL: https://github.com/apache/hive/pull/4356#issuecomment-1578364998
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=4356)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4356&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4356&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4356&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4356&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4356&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4356&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4356&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4356&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4356&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4356&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4356&resolved=false&types=CODE_SMELL) [10 Code Smells](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4356&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=4356&metric=coverage&view=list) No Coverage information
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=4356&metric=duplicated_lines_density&view=list) No Duplication information
--
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 diff in pull request #4356: HIVE-27373: Unable to pushdown partition filter with unquoted date literal type to metastore
Posted by "kasakrisz (via GitHub)" <gi...@apache.org>.
kasakrisz commented on code in PR #4356:
URL: https://github.com/apache/hive/pull/4356#discussion_r1214326066
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/parser/Filter.g:
##########
@@ -52,34 +53,36 @@ import java.util.regex.Pattern;
private static final Pattern datePattern = Pattern.compile(".*(\\d\\d\\d\\d-\\d\\d-\\d\\d).*");
private static final Pattern timestampPattern =
Pattern.compile(".*(\\d\\d\\d\\d-\\d\\d-\\d\\d \\d\\d:\\d\\d:\\d\\d).*");
- private static final ThreadLocal<SimpleDateFormat> dateFormat =
- new ThreadLocal<SimpleDateFormat>() {
- @Override
- protected SimpleDateFormat initialValue() {
- SimpleDateFormat val = new SimpleDateFormat("yyyy-MM-dd");
- val.setLenient(false); // Without this, 2020-20-20 becomes 2021-08-20.
Review Comment:
Added some negative tests and also changed the formatter's ResolverStyle to strict
--
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] sonarcloud[bot] commented on pull request #4356: HIVE-27373: Unable to pushdown partition filter with unquoted date literal type to metastore
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #4356:
URL: https://github.com/apache/hive/pull/4356#issuecomment-1579304679
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=4356)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4356&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4356&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4356&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4356&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4356&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4356&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4356&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4356&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4356&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4356&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4356&resolved=false&types=CODE_SMELL) [11 Code Smells](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4356&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=4356&metric=coverage&view=list) No Coverage information
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=4356&metric=duplicated_lines_density&view=list) No Duplication information
--
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 diff in pull request #4356: HIVE-27373: Unable to pushdown partition filter with unquoted date literal type to metastore
Posted by "kasakrisz (via GitHub)" <gi...@apache.org>.
kasakrisz commented on code in PR #4356:
URL: https://github.com/apache/hive/pull/4356#discussion_r1211741849
##########
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestPartFilterExprUtil.java:
##########
@@ -0,0 +1,74 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.hive.metastore;
+
+import org.apache.hadoop.hive.metastore.annotation.MetastoreUnitTest;
+import org.apache.hadoop.hive.metastore.api.MetaException;
+import org.apache.hadoop.hive.metastore.parser.ExpressionTree;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import static org.hamcrest.core.Is.is;
+import static org.hamcrest.MatcherAssert.assertThat;
+
+@Category(MetastoreUnitTest.class)
+public class TestPartFilterExprUtil {
+
+ @Test
+ public void testMakeExpressionTreeWhenDateLiteralTypeIsNotSpecified() throws MetaException {
+ ExpressionTree expressionTree = PartFilterExprUtil.makeExpressionTree("(j = 1990-11-10 or j = 1990-11-11 or j = 1990-11-12)");
+ assertThat(expressionTree.getRoot().toString(), is("TreeNode{lhs=TreeNode{lhs=LeafNode{keyName='j', operator='=', value=1990-11-10}, andOr='OR', rhs=LeafNode{keyName='j', operator='=', value=1990-11-11}}, andOr='OR', rhs=LeafNode{keyName='j', operator='=', value=1990-11-12}}"));
+ }
+
+ @Test
+ public void testMakeExpressionTreeWhenDateLiteralIsQuoted() throws MetaException {
+ ExpressionTree expressionTree = PartFilterExprUtil.makeExpressionTree("(j = '1990-11-10' or j = '1990-11-11' or j = '1990-11-12')");
Review Comment:
I changed the keywords to be optional and it enabled us to parse quoted literals to java Date and Timestamp
--
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] sonarcloud[bot] commented on pull request #4356: HIVE-27373: Unable to pushdown partition filter with unquoted date literal type to metastore
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #4356:
URL: https://github.com/apache/hive/pull/4356#issuecomment-1580275282
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=4356)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4356&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4356&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4356&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4356&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4356&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4356&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4356&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4356&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4356&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4356&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4356&resolved=false&types=CODE_SMELL) [10 Code Smells](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4356&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=4356&metric=coverage&view=list) No Coverage information
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=4356&metric=duplicated_lines_density&view=list) No Duplication information
--
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 #4356: HIVE-27373: Unable to pushdown partition filter with unquoted date literal type to metastore
Posted by "kasakrisz (via GitHub)" <gi...@apache.org>.
kasakrisz commented on PR #4356:
URL: https://github.com/apache/hive/pull/4356#issuecomment-1572497282
@wecharyu
In the shared screenshot I see that you are testing with Timestamp literals.
Could you please try with date literals too both on master and your branch?
```
ExpressionTree expressionTree = PartFilterExprUtil.getFilterParser("j = 1990-11-12 or j = 1990-11-11 or j = 1990-11-10").tree;
```
it is throwing exception for me on master
```
Exception in thread "main" MetaException(message:Error parsing partition filter; lexer error: line 1:15 mismatched character 'O' expecting set null; exception NoViableAltException(13@[]))
at org.apache.hadoop.hive.metastore.PartFilterExprUtil.getFilterParser(PartFilterExprUtil.java:134)
at org.apache.hadoop.hive.metastore.PartFilterExprUtil.main(PartFilterExprUtil.java:41)
```
--
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] sonarcloud[bot] commented on pull request #4356: HIVE-27373: Unable to pushdown partition filter with unquoted date literal type to metastore
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #4356:
URL: https://github.com/apache/hive/pull/4356#issuecomment-1573589480
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=4356)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4356&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4356&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4356&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4356&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4356&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4356&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4356&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4356&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4356&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4356&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4356&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4356&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=4356&metric=coverage&view=list) No Coverage information
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=4356&metric=duplicated_lines_density&view=list) No Duplication information
--
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 diff in pull request #4356: HIVE-27373: Unable to pushdown partition filter with unquoted date literal type to metastore
Posted by "kasakrisz (via GitHub)" <gi...@apache.org>.
kasakrisz commented on code in PR #4356:
URL: https://github.com/apache/hive/pull/4356#discussion_r1214326066
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/parser/Filter.g:
##########
@@ -52,34 +53,36 @@ import java.util.regex.Pattern;
private static final Pattern datePattern = Pattern.compile(".*(\\d\\d\\d\\d-\\d\\d-\\d\\d).*");
private static final Pattern timestampPattern =
Pattern.compile(".*(\\d\\d\\d\\d-\\d\\d-\\d\\d \\d\\d:\\d\\d:\\d\\d).*");
- private static final ThreadLocal<SimpleDateFormat> dateFormat =
- new ThreadLocal<SimpleDateFormat>() {
- @Override
- protected SimpleDateFormat initialValue() {
- SimpleDateFormat val = new SimpleDateFormat("yyyy-MM-dd");
- val.setLenient(false); // Without this, 2020-20-20 becomes 2021-08-20.
Review Comment:
Added some negative tests
--
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] zabetak commented on a diff in pull request #4356: HIVE-27373: Unable to pushdown partition filter with unquoted date literal type to metastore
Posted by "zabetak (via GitHub)" <gi...@apache.org>.
zabetak commented on code in PR #4356:
URL: https://github.com/apache/hive/pull/4356#discussion_r1211484999
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/parser/Filter.g:
##########
@@ -549,7 +549,6 @@ DateLiteral
TimestampLiteral
:
KW_TIMESTAMP '\'' TimestampString '\'' { extractTimestamp(getText()) != null }?
- | TimestampString { extractTimestamp(getText()) != null }?
Review Comment:
I am wondering if we could retain the support for unquoted literals and at the same time be backwards compatible by combining the `DateLiteral` and `TimestampLiteral` together.
```
DateTimeLiteral
:
KW_TIMESTAMP '\'' TimestampString '\'' { extractTimestamp(getText()) != null }?
| TimestampString { extractTimestamp(getText()) != null }?
| KW_DATE '\'' DateString '\'' { extractDate(getText()) != null }?
| DateString { extractDate(getText()) != null }?
;
```
This may also allow us to refactor and reduce duplication in other parser rules inside `Filter.g`.
Is it possible/ Does it make sense?
##########
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestPartFilterExprUtil.java:
##########
@@ -0,0 +1,74 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.hive.metastore;
+
+import org.apache.hadoop.hive.metastore.annotation.MetastoreUnitTest;
+import org.apache.hadoop.hive.metastore.api.MetaException;
+import org.apache.hadoop.hive.metastore.parser.ExpressionTree;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import static org.hamcrest.core.Is.is;
+import static org.hamcrest.MatcherAssert.assertThat;
+
+@Category(MetastoreUnitTest.class)
+public class TestPartFilterExprUtil {
+
+ @Test
+ public void testMakeExpressionTreeWhenDateLiteralTypeIsNotSpecified() throws MetaException {
+ ExpressionTree expressionTree = PartFilterExprUtil.makeExpressionTree("(j = 1990-11-10 or j = 1990-11-11 or j = 1990-11-12)");
+ assertThat(expressionTree.getRoot().toString(), is("TreeNode{lhs=TreeNode{lhs=LeafNode{keyName='j', operator='=', value=1990-11-10}, andOr='OR', rhs=LeafNode{keyName='j', operator='=', value=1990-11-11}}, andOr='OR', rhs=LeafNode{keyName='j', operator='=', value=1990-11-12}}"));
+ }
+
+ @Test
+ public void testMakeExpressionTreeWhenDateLiteralIsQuoted() throws MetaException {
+ ExpressionTree expressionTree = PartFilterExprUtil.makeExpressionTree("(j = '1990-11-10' or j = '1990-11-11' or j = '1990-11-12')");
+ assertThat(expressionTree.getRoot().toString(), is("TreeNode{lhs=TreeNode{lhs=LeafNode{keyName='j', operator='=', value=1990-11-10}, andOr='OR', rhs=LeafNode{keyName='j', operator='=', value=1990-11-11}}, andOr='OR', rhs=LeafNode{keyName='j', operator='=', value=1990-11-12}}"));
+ }
+
+ @Test
+ public void testMakeExpressionTreeWhenDateLiteralTypeIsSpecified() throws MetaException {
+ ExpressionTree expressionTree = PartFilterExprUtil.makeExpressionTree("(j) IN (DATE'1990-11-10', DATE'1990-11-11', DATE'1990-11-12')");
+ assertThat(expressionTree.getRoot().toString(), is("TreeNode{lhs=TreeNode{lhs=LeafNode{keyName='j', operator='=', value=1990-11-10}, andOr='OR', rhs=LeafNode{keyName='j', operator='=', value=1990-11-11}}, andOr='OR', rhs=LeafNode{keyName='j', operator='=', value=1990-11-12}}"));
+ }
+
+ @Test
+ public void testMakeExpressionTreeWhenTimestampLiteralTypeIsSpecified() throws MetaException {
+ ExpressionTree expressionTree = PartFilterExprUtil.makeExpressionTree("(j) IN (TIMESTAMP'2000-01-01 01:00:00', TIMESTAMP'2000-01-01 01:42:00')");
+ assertThat(expressionTree.getRoot().toString(), is("TreeNode{lhs=LeafNode{keyName='j', operator='=', value=2000-01-01 01:00:00.0}, andOr='OR', rhs=LeafNode{keyName='j', operator='=', value=2000-01-01 01:42:00.0}}"));
+ }
+
+ @Test
+ public void testMakeExpressionTreeWhenTimestampLiteralIsQuoted() throws MetaException {
Review Comment:
Even if we decide to drop support for unquoted timestamp literals we may want to add a test case that asserts that an exception is raised in that case.
##########
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestPartFilterExprUtil.java:
##########
@@ -0,0 +1,74 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.hive.metastore;
+
+import org.apache.hadoop.hive.metastore.annotation.MetastoreUnitTest;
+import org.apache.hadoop.hive.metastore.api.MetaException;
+import org.apache.hadoop.hive.metastore.parser.ExpressionTree;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import static org.hamcrest.core.Is.is;
+import static org.hamcrest.MatcherAssert.assertThat;
+
+@Category(MetastoreUnitTest.class)
+public class TestPartFilterExprUtil {
+
+ @Test
+ public void testMakeExpressionTreeWhenDateLiteralTypeIsNotSpecified() throws MetaException {
+ ExpressionTree expressionTree = PartFilterExprUtil.makeExpressionTree("(j = 1990-11-10 or j = 1990-11-11 or j = 1990-11-12)");
+ assertThat(expressionTree.getRoot().toString(), is("TreeNode{lhs=TreeNode{lhs=LeafNode{keyName='j', operator='=', value=1990-11-10}, andOr='OR', rhs=LeafNode{keyName='j', operator='=', value=1990-11-11}}, andOr='OR', rhs=LeafNode{keyName='j', operator='=', value=1990-11-12}}"));
+ }
+
+ @Test
+ public void testMakeExpressionTreeWhenDateLiteralIsQuoted() throws MetaException {
+ ExpressionTree expressionTree = PartFilterExprUtil.makeExpressionTree("(j = '1990-11-10' or j = '1990-11-11' or j = '1990-11-12')");
Review Comment:
This is parsed as a `StringLiteral` right? Probably we don't care but don't know if we want to make this visible/assertable (inventing words) in the test.
##########
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestPartFilterExprUtil.java:
##########
@@ -0,0 +1,74 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.hive.metastore;
+
+import org.apache.hadoop.hive.metastore.annotation.MetastoreUnitTest;
+import org.apache.hadoop.hive.metastore.api.MetaException;
+import org.apache.hadoop.hive.metastore.parser.ExpressionTree;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import static org.hamcrest.core.Is.is;
+import static org.hamcrest.MatcherAssert.assertThat;
+
+@Category(MetastoreUnitTest.class)
+public class TestPartFilterExprUtil {
Review Comment:
In terms of test coverage we may want to add a test for each category of operator expressions:
```
operatorExpression
:
betweenExpression
|
inExpression
|
multiColInExpression
|
binOpExpression
;
```
So far I see that we have for `inExpression` and `binOpExpression` but not for the rest.
Basically I would suggest the following matrix:
operator expressions X date variants + operator expressions X timestamp variants = 4 X 3 + 4 X 3 = 24 tests in total; optionally we can mix in some AND/ORs if we want.
Making the test parameterized may reduce some boiler plate but not a must do just a simple suggestion.
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/PartFilterExprUtil.java:
##########
@@ -101,7 +102,8 @@ public static PartitionExpressionProxy createExpressionProxy(Configuration conf)
* @param filter Filter.
* @return Expression tree. Null if there was an error.
*/
- private static ExpressionTree makeExpressionTree(String filter) throws MetaException {
+ @VisibleForTesting
+ static ExpressionTree makeExpressionTree(String filter) throws MetaException {
Review Comment:
Instead of changing the visibility here maybe we could just use `FilterParser getFilterParser(String filter)` in the unit test, which is already public.
##########
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestPartFilterExprUtil.java:
##########
@@ -0,0 +1,74 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.hive.metastore;
+
+import org.apache.hadoop.hive.metastore.annotation.MetastoreUnitTest;
+import org.apache.hadoop.hive.metastore.api.MetaException;
+import org.apache.hadoop.hive.metastore.parser.ExpressionTree;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import static org.hamcrest.core.Is.is;
+import static org.hamcrest.MatcherAssert.assertThat;
+
+@Category(MetastoreUnitTest.class)
+public class TestPartFilterExprUtil {
Review Comment:
Since we are focusing on dates and timestamps maybe we should rename the class accordingly; don't feel strongly about it do as you feel.
--
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] wecharyu commented on pull request #4356: HIVE-27373: Unable to pushdown partition filter with unquoted date literal type to metastore
Posted by "wecharyu (via GitHub)" <gi...@apache.org>.
wecharyu commented on PR #4356:
URL: https://github.com/apache/hive/pull/4356#issuecomment-1573062587
@kasakrisz You are correct. The date literals will cause parse exception in master. Luckily they work fine in my branch.
--
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] wecharyu commented on pull request #4356: HIVE-27373: Unable to pushdown partition filter with unquoted date literal type to metastore
Posted by "wecharyu (via GitHub)" <gi...@apache.org>.
wecharyu commented on PR #4356:
URL: https://github.com/apache/hive/pull/4356#issuecomment-1572156584
I have pushed a PR #4344 to support ANTLR4 for `Filter.g`, and it has supported the unquoted date literal type, can you also take a look? @kasakrisz @zabetak
--
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 #4356: HIVE-27373: Unable to pushdown partition filter with unquoted date literal type to metastore
Posted by "kasakrisz (via GitHub)" <gi...@apache.org>.
kasakrisz merged PR #4356:
URL: https://github.com/apache/hive/pull/4356
--
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 diff in pull request #4356: HIVE-27373: Unable to pushdown partition filter with unquoted date literal type to metastore
Posted by "kasakrisz (via GitHub)" <gi...@apache.org>.
kasakrisz commented on code in PR #4356:
URL: https://github.com/apache/hive/pull/4356#discussion_r1219185871
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/parser/Filter.g:
##########
@@ -37,6 +37,7 @@ package org.apache.hadoop.hive.metastore.parser;
import java.sql.Date;
import java.sql.Timestamp;
+import java.time.LocalDate;
import java.time.LocalDateTime;
import java.time.format.DateTimeFormatter;
import java.text.ParseException;
Review Comment:
I just checked and removed these. Thanks.
--
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] sonarcloud[bot] commented on pull request #4356: HIVE-27373: Unable to pushdown partition filter with unquoted date literal type to metastore
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #4356:
URL: https://github.com/apache/hive/pull/4356#issuecomment-1570403166
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=4356)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4356&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4356&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4356&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4356&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4356&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4356&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4356&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4356&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4356&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4356&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4356&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4356&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=4356&metric=coverage&view=list) No Coverage information
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=4356&metric=duplicated_lines_density&view=list) No Duplication information
--
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 diff in pull request #4356: HIVE-27373: Unable to pushdown partition filter with unquoted date literal type to metastore
Posted by "kasakrisz (via GitHub)" <gi...@apache.org>.
kasakrisz commented on code in PR #4356:
URL: https://github.com/apache/hive/pull/4356#discussion_r1211743929
##########
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestPartFilterExprUtil.java:
##########
@@ -0,0 +1,74 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.hive.metastore;
+
+import org.apache.hadoop.hive.metastore.annotation.MetastoreUnitTest;
+import org.apache.hadoop.hive.metastore.api.MetaException;
+import org.apache.hadoop.hive.metastore.parser.ExpressionTree;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import static org.hamcrest.core.Is.is;
+import static org.hamcrest.MatcherAssert.assertThat;
+
+@Category(MetastoreUnitTest.class)
+public class TestPartFilterExprUtil {
Review Comment:
I added some tests to Int and String so I kept the original name of the class.
--
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] zabetak commented on a diff in pull request #4356: HIVE-27373: Unable to pushdown partition filter with unquoted date literal type to metastore
Posted by "zabetak (via GitHub)" <gi...@apache.org>.
zabetak commented on code in PR #4356:
URL: https://github.com/apache/hive/pull/4356#discussion_r1214004385
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/parser/Filter.g:
##########
@@ -52,34 +53,36 @@ import java.util.regex.Pattern;
private static final Pattern datePattern = Pattern.compile(".*(\\d\\d\\d\\d-\\d\\d-\\d\\d).*");
private static final Pattern timestampPattern =
Pattern.compile(".*(\\d\\d\\d\\d-\\d\\d-\\d\\d \\d\\d:\\d\\d:\\d\\d).*");
- private static final ThreadLocal<SimpleDateFormat> dateFormat =
- new ThreadLocal<SimpleDateFormat>() {
- @Override
- protected SimpleDateFormat initialValue() {
- SimpleDateFormat val = new SimpleDateFormat("yyyy-MM-dd");
- val.setLenient(false); // Without this, 2020-20-20 becomes 2021-08-20.
Review Comment:
The old date parser was STRICT and the timestamp parser was `ResolverStyle.SMART`(the default).
The new parsers are both `ResolverStyle.SMART`. In other parts of Hive there is a move to slowly shift date/timestamp parsers to be STRICT so it may make sense to keep them strict here as well.
If we want to keep the old behavior we can do that as well but in either case let's add a few tests with invalid/out-of-range dates & timestamps.
```
2023-06-02 99:35:00
2023-06-32 10:35:00
2023-06-32
```
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/parser/Filter.g:
##########
@@ -52,34 +53,36 @@ import java.util.regex.Pattern;
private static final Pattern datePattern = Pattern.compile(".*(\\d\\d\\d\\d-\\d\\d-\\d\\d).*");
private static final Pattern timestampPattern =
Pattern.compile(".*(\\d\\d\\d\\d-\\d\\d-\\d\\d \\d\\d:\\d\\d:\\d\\d).*");
- private static final ThreadLocal<SimpleDateFormat> dateFormat =
- new ThreadLocal<SimpleDateFormat>() {
- @Override
- protected SimpleDateFormat initialValue() {
- SimpleDateFormat val = new SimpleDateFormat("yyyy-MM-dd");
- val.setLenient(false); // Without this, 2020-20-20 becomes 2021-08-20.
- val.setTimeZone(TimeZone.getTimeZone("UTC"));
- return val;
- };
- };
- private static final ThreadLocal<DateTimeFormatter> timestampFormat =
- new ThreadLocal<DateTimeFormatter>() {
- @Override
- protected DateTimeFormatter initialValue() {
- DateTimeFormatter val = DateTimeFormatter.ofPattern("yyyy-MM-dd HH:mm:ss")
- .withZone(TimeZone.getTimeZone("UTC").toZoneId());
- return val;
- };
- };
- public static java.sql.Date extractDate(String input) {
+ private static final ThreadLocal<DateTimeFormatter> dateFormat = createDateTimeFormatter("yyyy-MM-dd");
+ private static final ThreadLocal<DateTimeFormatter> timestampFormat = createDateTimeFormatter("yyyy-MM-dd HH:mm:ss");
+
+ private static ThreadLocal<DateTimeFormatter> createDateTimeFormatter(String format) {
+ return new ThreadLocal<DateTimeFormatter>() {
+ @Override
+ protected DateTimeFormatter initialValue() {
+ DateTimeFormatter val = DateTimeFormatter.ofPattern(format)
+ .withZone(TimeZone.getTimeZone("UTC").toZoneId());
Review Comment:
Since we are using `LocalDate` and `LocalDateTime` I don't know how much sense it makes to use a zone here. However, it was like that before so we can keep it as is.
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/parser/Filter.g:
##########
@@ -549,7 +549,6 @@ DateLiteral
TimestampLiteral
:
KW_TIMESTAMP '\'' TimestampString '\'' { extractTimestamp(getText()) != null }?
- | TimestampString { extractTimestamp(getText()) != null }?
Review Comment:
I now understand better the problem with the conflict; I forgot that rule order does not matter.
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/parser/Filter.g:
##########
@@ -37,6 +37,7 @@ package org.apache.hadoop.hive.metastore.parser;
import java.sql.Date;
import java.sql.Timestamp;
+import java.time.LocalDate;
import java.time.LocalDateTime;
import java.time.format.DateTimeFormatter;
import java.text.ParseException;
Review Comment:
Unused imports?
##########
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestPartFilterExprUtil.java:
##########
@@ -0,0 +1,74 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.hive.metastore;
+
+import org.apache.hadoop.hive.metastore.annotation.MetastoreUnitTest;
+import org.apache.hadoop.hive.metastore.api.MetaException;
+import org.apache.hadoop.hive.metastore.parser.ExpressionTree;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import static org.hamcrest.core.Is.is;
+import static org.hamcrest.MatcherAssert.assertThat;
+
+@Category(MetastoreUnitTest.class)
+public class TestPartFilterExprUtil {
Review Comment:
Actually I didn't have in mind generating the expressions but having parameters as {expression, expectedTree} pairs.
```
new Object[] {"(j) in (1990-11-10)", "LeafNode{keyName='j', operator='=', value=1990-11-10}"},
new Object[] {"(j) IN (DATE'1990-11-10')", "LeafNode{keyName='j', operator='=', value=1990-11-10}"},
...
```
Parameters could also be loaded from a CSV file which could further cut down the need for double quotes etc (https://junit.org/junit5/docs/snapshot/user-guide/#writing-tests-parameterized-tests-sources-CsvFileSource):
```
(j) in (1990-11-10); LeafNode{keyName='j', operator='=', value=1990-11-10}
(j) IN (DATE'1990-11-10'); LeafNode{keyName='j', operator='=', value=1990-11-10}
```
Anyways the test is completely fine as it is and I like that its name denotes exactly what it does. I just wanted to share what I had in mind.
--
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] zabetak commented on a diff in pull request #4356: HIVE-27373: Unable to pushdown partition filter with unquoted date literal type to metastore
Posted by "zabetak (via GitHub)" <gi...@apache.org>.
zabetak commented on code in PR #4356:
URL: https://github.com/apache/hive/pull/4356#discussion_r1218172896
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/parser/Filter.g:
##########
@@ -37,6 +37,7 @@ package org.apache.hadoop.hive.metastore.parser;
import java.sql.Date;
import java.sql.Timestamp;
+import java.time.LocalDate;
import java.time.LocalDateTime;
import java.time.format.DateTimeFormatter;
import java.text.ParseException;
Review Comment:
I don't see a reference to `ParseException` and `SimpleDateFormat` so I was thinking that the respective imports are redundant but I may be missing something; not big deal anyways.
--
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 diff in pull request #4356: HIVE-27373: Unable to pushdown partition filter with unquoted date literal type to metastore
Posted by "kasakrisz (via GitHub)" <gi...@apache.org>.
kasakrisz commented on code in PR #4356:
URL: https://github.com/apache/hive/pull/4356#discussion_r1214325527
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/parser/Filter.g:
##########
@@ -37,6 +37,7 @@ package org.apache.hadoop.hive.metastore.parser;
import java.sql.Date;
import java.sql.Timestamp;
+import java.time.LocalDate;
import java.time.LocalDateTime;
import java.time.format.DateTimeFormatter;
import java.text.ParseException;
Review Comment:
It is necessary in `extractDate`
```
try {
LocalDate val = LocalDate.parse(m.group(1), dateFormat);
return java.sql.Date.valueOf(val);
} catch (Exception ex) {
```
--
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] sonarcloud[bot] commented on pull request #4356: HIVE-27373: Unable to pushdown partition filter with unquoted date literal type to metastore
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #4356:
URL: https://github.com/apache/hive/pull/4356#issuecomment-1563122633
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=4356)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4356&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4356&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4356&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4356&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4356&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4356&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4356&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4356&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4356&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4356&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4356&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4356&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=4356&metric=coverage&view=list) No Coverage information
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=4356&metric=duplicated_lines_density&view=list) No Duplication information
--
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