You are viewing a plain text version of this content. The canonical link for it is here.
Posted to gitbox@hive.apache.org by "mdayakar (via GitHub)" <gi...@apache.org> on 2023/09/29 17:11:25 UTC
[GitHub] [hive] mdayakar opened a new pull request, #4760: HIVE-27760: Filter on date type partitioning column producing 0 results
mdayakar opened a new pull request, #4760:
URL: https://github.com/apache/hive/pull/4760
<!--
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?
**RootCause:** As a part of [HIVE-27373](http://example.com/) issue fix to parse the string to java.sql.Date object, java.text.SimpleDateFormat is replaced with java.time.format.DateTimeFormatter using java.time.LocalDate which represents a Date without TimeZone. Here this input is passed [here ](https://github.com/apache/hive/blob/master/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/MetaStoreDirectSql.java#L1370) which uses SimpleDateFormat(parsing dates in a locale-sensitive manner) and java.sql.Date. Here user timezone is passed different so actual value is getting changed to a different value (for example 2023-01-01 is changed to 2022-12-31) which is not matching with any partition so nothing gets returned.
**Solution:** In MetaStoreDirectSql.java, we should use java.time.format.DateTimeFormatter with java.time.LocalDate so that it will return proper date string.
<!--
Please clarify what changes you are proposing. The purpose of this section is to outline the changes and how this PR fixes the issue.
If possible, please consider writing useful notes for better and faster reviews in your PR. See the examples below.
1. If you refactor some codes with changing classes, showing the class hierarchy will help reviewers.
2. If you fix some SQL features, you can provide some references of other DBMSes.
3. If there is design documentation, please add the link.
4. If there is a discussion in the mailing list, please add the link.
-->
### Why are the changes needed?
To fix HIVE-27760 otherwise it will results-in wrong results when Date/Timestamp partition column is used in a filter.
<!--
Please clarify why the changes are needed. For instance,
1. If you propose a new API, clarify the use case for a new API.
2. If you fix a bug, you can clarify why it is a bug.
-->
### Does this PR introduce _any_ user-facing change?
No
<!--
Note that it means *any* user-facing change including all aspects such as the documentation fix.
If yes, please clarify the previous behavior and the change this PR proposes - provide the console output, description, screenshot and/or a reproducable example to show the behavior difference if possible.
If possible, please also clarify if this is a user-facing change compared to the released Hive versions or within the unreleased branches such as master.
If no, write 'No'.
-->
### Is the change a dependency upgrade?
No
<!--
If yes, please attach a file with output from mvn dependency:tree to validate a complete upgrade of dependency.
If no, write 'No'.
-->
### How was this patch tested?
Unit tests and q test
_mvn test -Dtest.output.overwrite -Dtest=TestMiniLlapLocalCliDriver -Dqfile=date_timestamp_partition_filter.q -pl itests/qtest -Pitests_
<!--
If tests were added, say they were added here. Please make sure to add some test cases that check the changes thoroughly including negative and positive cases if possible.
If it was tested in a way different from regular unit tests, please clarify how you tested step by step, ideally copy and paste-able, so that other reviewers can test and check, and descendants can verify in the future.
If tests were not added, please describe why they were not added and/or why it was difficult to add.
-->
--
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
Re: [PR] HIVE-27760: Filter on date type partitioning column producing 0 results [hive]
Posted by "mdayakar (via GitHub)" <gi...@apache.org>.
mdayakar commented on code in PR #4760:
URL: https://github.com/apache/hive/pull/4760#discussion_r1343813888
##########
ql/src/java/org/apache/hadoop/hive/ql/parse/BaseSemanticAnalyzer.java:
##########
@@ -1804,9 +1803,9 @@ private static String normalizeDateCol(Object colValue) throws SemanticException
throw new SemanticException("Unexpected date type " + colValue.getClass());
}
try {
- return MetaStoreUtils.PARTITION_DATE_FORMAT.get().format(
- MetaStoreUtils.PARTITION_DATE_FORMAT.get().parse(value.toString()));
- } catch (ParseException e) {
+ return MetaStoreUtils.convertDateToString(
+ MetaStoreUtils.convertStringToDate(value.toString()));
+ } catch (Exception e) {
throw new SemanticException(e);
}
}
Review Comment:
In case of `ParseException`, they were catching it and throwing `SemanticException`, now new code don't throw any exception in normal scenario so for the safer side catching `Exception` and throwing back `SemanticException`.
--
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
Re: [PR] HIVE-27760: Filter on date type partitioning column producing 0 results [hive]
Posted by "mdayakar (via GitHub)" <gi...@apache.org>.
mdayakar commented on code in PR #4760:
URL: https://github.com/apache/hive/pull/4760#discussion_r1346957855
##########
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/utils/TestMetaStoreUtils.java:
##########
@@ -0,0 +1,78 @@
+/*
+ * 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
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * 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.utils;
+
+import org.apache.hadoop.hive.metastore.annotation.MetastoreUnitTest;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import java.sql.Date;
+import java.sql.Timestamp;
+import java.time.ZoneId;
+import java.util.TimeZone;
+
+import static org.junit.Assert.assertEquals;
+
+@Category(MetastoreUnitTest.class)
+public class TestMetaStoreUtils {
Review Comment:
Removed string -> date/timestamp -> string test cases as per comment in the commit (https://github.com/apache/hive/commit/4ff9ba968b31ea16611ba5d5bc05bbfb3b4fe184).
--
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
Re: [PR] HIVE-27760: Filter on date type partitioning column producing 0 results [hive]
Posted by "mdayakar (via GitHub)" <gi...@apache.org>.
mdayakar commented on code in PR #4760:
URL: https://github.com/apache/hive/pull/4760#discussion_r1347063916
##########
ql/src/test/results/clientpositive/llap/date_timestamp_partition_filter.q.out:
##########
@@ -0,0 +1,214 @@
+PREHOOK: query: CREATE EXTERNAL TABLE testpd(col1 string, col2 String) PARTITIONED BY(PartitionDate DATE) STORED AS ORC
Review Comment:
There is no change in the EXPLAIN plan output, I just added to make sure partition filter pushdown is happening properly.
--
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
Re: [PR] HIVE-27760: Filter on date type partitioning column producing 0 results [hive]
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #4760:
URL: https://github.com/apache/hive/pull/4760#issuecomment-1747717071
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=4760)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4760&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4760&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4760&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4760&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4760&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4760&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4760&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4760&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4760&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4760&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4760&resolved=false&types=CODE_SMELL) [3 Code Smells](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4760&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=4760&metric=coverage&view=list) No Coverage information
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=4760&metric=duplicated_lines_density&view=list) No Duplication information
 The version of Java (11.0.8) you have used to run this analysis is deprecated and we will stop accepting it soon. Please update to at least Java 17.
Read more [here](https://docs.sonarcloud.io/appendices/scanner-environment/)
--
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
Re: [PR] HIVE-27760: Filter on date type partitioning column producing 0 results [hive]
Posted by "mdayakar (via GitHub)" <gi...@apache.org>.
mdayakar commented on code in PR #4760:
URL: https://github.com/apache/hive/pull/4760#discussion_r1344540657
##########
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/utils/TestMetaStoreUtils.java:
##########
@@ -0,0 +1,78 @@
+/*
+ * 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
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * 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.utils;
+
+import org.apache.hadoop.hive.metastore.annotation.MetastoreUnitTest;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import java.sql.Date;
+import java.sql.Timestamp;
+import java.time.ZoneId;
+import java.util.TimeZone;
+
+import static org.junit.Assert.assertEquals;
+
+@Category(MetastoreUnitTest.class)
+public class TestMetaStoreUtils {
Review Comment:
> The issue was coming when SimpleDateFormat and java.sql.Date is used.
Let me rephrase my question cause it was not completely clear. Can someone see the problem after switching the formatter behind MetaStoreUtils.convertDateToString using the unit tests here?
Yes, the testcase fails when someone changes the formatter back to SimpleDateFormat. For example if I change like below
```java
public static String convertDateToString(Date date) {
DateFormat 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.format(date);
//return DATE_FORMATTER.format(date.toLocalDate());
}
```
then the testcases are failing
```
[ERROR] Failures:
[ERROR] TestMetaStoreUtils.testConvertDateToString:46 expected:<202[3-01-0]1> but was:<202[2-12-3]1>
[ERROR] TestMetaStoreUtils.testConvertStringToDateToString:72 expected:<202[3-01-0]1> but was:<202[2-12-3]1>
[INFO]
[ERROR] Tests run: 6, Failures: 2, Errors: 0, Skipped: 0
```
--
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
Re: [PR] HIVE-27760: Filter on date type partitioning column producing 0 results [hive]
Posted by "mdayakar (via GitHub)" <gi...@apache.org>.
mdayakar commented on code in PR #4760:
URL: https://github.com/apache/hive/pull/4760#discussion_r1346968329
##########
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/utils/TestMetaStoreUtils.java:
##########
@@ -0,0 +1,78 @@
+/*
+ * 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
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * 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.utils;
+
+import org.apache.hadoop.hive.metastore.annotation.MetastoreUnitTest;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import java.sql.Date;
+import java.sql.Timestamp;
+import java.time.ZoneId;
+import java.util.TimeZone;
+
+import static org.junit.Assert.assertEquals;
+
+@Category(MetastoreUnitTest.class)
+public class TestMetaStoreUtils {
+
+ @Test
+ public void testConvertDateToString() {
+ TimeZone defaultTZ = TimeZone.getDefault();
+ try {
+ TimeZone.setDefault(TimeZone.getTimeZone(ZoneId.of("Asia/Hong_Kong")));
+ String date = MetaStoreUtils.convertDateToString(Date.valueOf("2023-01-01"));
+ assertEquals("2023-01-01", date);
+ } finally {
+ TimeZone.setDefault(defaultTZ);
+ }
+ }
+
+ @Test
+ public void testcConvertTimestampToString() {
+ TimeZone defaultTZ = TimeZone.getDefault();
+ try {
+ String date = MetaStoreUtils.convertTimestampToString(Timestamp.valueOf("2023-01-01 10:20:30"));
+ assertEquals("2023-01-01 10:20:30", date);
+ } finally {
+ TimeZone.setDefault(defaultTZ);
+ }
Review Comment:
Thanks for the commit (https://github.com/apache/hive/commit/4ff9ba968b31ea16611ba5d5bc05bbfb3b4fe184) @zabetak , changed the test code as per your commit. Just added negative testcases when invalid date/timestamp is passed to the APIs.
> With the new tests, I found a bug in convertTimestampToString using [zoneId=Europe/Paris, timestamp=9743-03-31T03:37:33] but I think this existed since day one so we can possibly defer it to another PR.
I found this issue is not with our code, the problem is coming from `java.sql.Timestamp.valueOf()` API. Below are some observations, here for time less than 4hrs it is adding +1 as timezone and for 3hrs it is also changing 03hrs to 02hrs. Not sure exactly why it is happening. We may need to check further.
```
Input String: 9743-03-31 05:37:33
result = {Timestamp@3282} "9743-03-31 05:37:33.0"
nanos = 0
fastTime = 245299894653000
cdate = {Gregorian$Date@3284} "9743-03-31T05:37:33.000+0200"
Input String: 9743-03-31 04:37:33
result = {Timestamp@3236} "9743-03-31 04:37:33.0"
nanos = 0
fastTime = 245299891053000
cdate = {Gregorian$Date@3238} "9743-03-31T04:37:33.000+0200"
Input String: 9743-03-31 03:37:33
result = {Timestamp@3244} "9743-03-31 02:37:33.0"
nanos = 0
fastTime = 245299887453000
cdate = {Gregorian$Date@3246} "9743-03-31T02:37:33.000+0100"
Input String: 9743-03-31 02:37:33
result = {Timestamp@3274} "9743-03-31 02:37:33.0"
nanos = 0
fastTime = 245299887453000
cdate = {Gregorian$Date@3276} "9743-03-31T02:37:33.000+0100"
Input String: 9743-03-31 01:37:33
result = {Timestamp@3289} "9743-03-31 01:37:33.0"
nanos = 0
fastTime = 245299883853000
cdate = {Gregorian$Date@3291} "9743-03-31T01:37:33.000+0100"
```
--
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
Re: [PR] HIVE-27760: Filter on date type partitioning column producing 0 results [hive]
Posted by "mdayakar (via GitHub)" <gi...@apache.org>.
mdayakar commented on code in PR #4760:
URL: https://github.com/apache/hive/pull/4760#discussion_r1343808097
##########
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/utils/TestMetaStoreUtils.java:
##########
@@ -0,0 +1,78 @@
+/*
+ * 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
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * 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.utils;
+
+import org.apache.hadoop.hive.metastore.annotation.MetastoreUnitTest;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import java.sql.Date;
+import java.sql.Timestamp;
+import java.time.ZoneId;
+import java.util.TimeZone;
+
+import static org.junit.Assert.assertEquals;
+
+@Category(MetastoreUnitTest.class)
+public class TestMetaStoreUtils {
Review Comment:
Would some of these tests fail without the new changes introduced in this PR?
--> The issue was coming when SimpleDateFormat and java.sql.Date is used.
Can we also add tests for:
String -> Date -> String
String -> Timestamp -> String
--> Added the tests for the same.
--
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
Re: [PR] HIVE-27760: Filter on date type partitioning column producing 0 results [hive]
Posted by "mdayakar (via GitHub)" <gi...@apache.org>.
mdayakar commented on code in PR #4760:
URL: https://github.com/apache/hive/pull/4760#discussion_r1345337431
##########
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/utils/TestMetaStoreUtils.java:
##########
@@ -0,0 +1,78 @@
+/*
+ * 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
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * 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.utils;
+
+import org.apache.hadoop.hive.metastore.annotation.MetastoreUnitTest;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import java.sql.Date;
+import java.sql.Timestamp;
+import java.time.ZoneId;
+import java.util.TimeZone;
+
+import static org.junit.Assert.assertEquals;
+
+@Category(MetastoreUnitTest.class)
+public class TestMetaStoreUtils {
Review Comment:
> I am mostly interested for the string -> date/timestamp -> string case.
Added testcases for the same please check.
--
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
Re: [PR] HIVE-27760: Filter on date type partitioning column producing 0 results [hive]
Posted by "zabetak (via GitHub)" <gi...@apache.org>.
zabetak commented on code in PR #4760:
URL: https://github.com/apache/hive/pull/4760#discussion_r1343951379
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/parser/ExpressionTree.java:
##########
@@ -18,7 +18,7 @@
package org.apache.hadoop.hive.metastore.parser;
import java.sql.Timestamp;
-import java.util.Date;
Review Comment:
The import was introduced by HIVE-5679 and you are right it was done accidentally.
##########
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/utils/TestMetaStoreUtils.java:
##########
@@ -0,0 +1,78 @@
+/*
+ * 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
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * 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.utils;
+
+import org.apache.hadoop.hive.metastore.annotation.MetastoreUnitTest;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import java.sql.Date;
+import java.sql.Timestamp;
+import java.time.ZoneId;
+import java.util.TimeZone;
+
+import static org.junit.Assert.assertEquals;
+
+@Category(MetastoreUnitTest.class)
+public class TestMetaStoreUtils {
Review Comment:
Can we parameterize the tests using more input literals (date/timestamps) and timezones to improve test coverage? You can draw some inspiration from: `org.apache.hadoop.hive.ql.io.parquet.serde.TestParquetTimestampsHive2Compatibility`
I am mostly interested for the `string -> date/timestamp -> string` case.
I had the impression that I already left that comment but it appears I forgot about it in the previous review round.
##########
ql/src/java/org/apache/hadoop/hive/ql/parse/BaseSemanticAnalyzer.java:
##########
@@ -1804,9 +1803,9 @@ private static String normalizeDateCol(Object colValue) throws SemanticException
throw new SemanticException("Unexpected date type " + colValue.getClass());
}
try {
- return MetaStoreUtils.PARTITION_DATE_FORMAT.get().format(
- MetaStoreUtils.PARTITION_DATE_FORMAT.get().parse(value.toString()));
- } catch (ParseException e) {
+ return MetaStoreUtils.convertDateToString(
+ MetaStoreUtils.convertStringToDate(value.toString()));
+ } catch (Exception e) {
throw new SemanticException(e);
}
}
Review Comment:
By dead code, I was referring to the whole `normalizeDateCol` method. It was not really about the changes in this PR.
##########
ql/src/test/results/clientpositive/llap/date_timestamp_partition_filter.q.out:
##########
@@ -0,0 +1,214 @@
+PREHOOK: query: CREATE EXTERNAL TABLE testpd(col1 string, col2 String) PARTITIONED BY(PartitionDate DATE) STORED AS ORC
Review Comment:
This question is still pending.
##########
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/utils/TestMetaStoreUtils.java:
##########
@@ -0,0 +1,78 @@
+/*
+ * 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
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * 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.utils;
+
+import org.apache.hadoop.hive.metastore.annotation.MetastoreUnitTest;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import java.sql.Date;
+import java.sql.Timestamp;
+import java.time.ZoneId;
+import java.util.TimeZone;
+
+import static org.junit.Assert.assertEquals;
+
+@Category(MetastoreUnitTest.class)
+public class TestMetaStoreUtils {
+
+ @Test
+ public void testConvertDateToString() {
+ TimeZone defaultTZ = TimeZone.getDefault();
+ try {
+ TimeZone.setDefault(TimeZone.getTimeZone(ZoneId.of("Asia/Hong_Kong")));
+ String date = MetaStoreUtils.convertDateToString(Date.valueOf("2023-01-01"));
+ assertEquals("2023-01-01", date);
+ } finally {
+ TimeZone.setDefault(defaultTZ);
+ }
+ }
+
+ @Test
+ public void testcConvertTimestampToString() {
+ TimeZone defaultTZ = TimeZone.getDefault();
+ try {
+ String date = MetaStoreUtils.convertTimestampToString(Timestamp.valueOf("2023-01-01 10:20:30"));
+ assertEquals("2023-01-01 10:20:30", date);
+ } finally {
+ TimeZone.setDefault(defaultTZ);
+ }
Review Comment:
Since all tests are switching timezone it would be nice to do some refactoring to avoid the boilerplate. Using `@Before`, `@After` annotated methods and/or `@Parameterized` tests could aid in this.
##########
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/utils/TestMetaStoreUtils.java:
##########
@@ -0,0 +1,78 @@
+/*
+ * 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
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * 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.utils;
+
+import org.apache.hadoop.hive.metastore.annotation.MetastoreUnitTest;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import java.sql.Date;
+import java.sql.Timestamp;
+import java.time.ZoneId;
+import java.util.TimeZone;
+
+import static org.junit.Assert.assertEquals;
+
+@Category(MetastoreUnitTest.class)
+public class TestMetaStoreUtils {
Review Comment:
> The issue was coming when SimpleDateFormat and java.sql.Date is used.
Let me rephrase my question cause it was not completely clear. Can someone see the problem after switching the formatter behind `MetaStoreUtils.convertDateToString` using the unit tests here?
--
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
Re: [PR] HIVE-27760: Filter on date type partitioning column producing 0 results [hive]
Posted by "mdayakar (via GitHub)" <gi...@apache.org>.
mdayakar commented on code in PR #4760:
URL: https://github.com/apache/hive/pull/4760#discussion_r1346983864
##########
ql/src/test/results/clientpositive/llap/date_timestamp_partition_filter.q.out:
##########
@@ -0,0 +1,214 @@
+PREHOOK: query: CREATE EXTERNAL TABLE testpd(col1 string, col2 String) PARTITIONED BY(PartitionDate DATE) STORED AS ORC
Review Comment:
Before fix the [output](https://github.com/apache/hive/pull/4760/files#diff-a03353b02b81f6c2c36cc130d0d85b019fb725c7cb0d57d0baa9b7122177610eR107) was empty, after fix it is returning the corresponding partition matching rows.
--
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
Re: [PR] HIVE-27760: Filter on date type partitioning column producing 0 results [hive]
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #4760:
URL: https://github.com/apache/hive/pull/4760#issuecomment-1749404003
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=4760)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4760&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4760&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4760&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4760&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4760&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4760&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4760&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4760&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4760&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4760&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4760&resolved=false&types=CODE_SMELL) [13 Code Smells](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4760&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=4760&metric=coverage&view=list) No Coverage information
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=4760&metric=duplicated_lines_density&view=list) No Duplication information
 The version of Java (11.0.8) you have used to run this analysis is deprecated and we will stop accepting it soon. Please update to at least Java 17.
Read more [here](https://docs.sonarcloud.io/appendices/scanner-environment/)
--
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
Re: [PR] HIVE-27760: Filter on date type partitioning column producing 0 results [hive]
Posted by "zabetak (via GitHub)" <gi...@apache.org>.
zabetak closed pull request #4760: HIVE-27760: Filter on date type partitioning column producing 0 results
URL: https://github.com/apache/hive/pull/4760
--
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
Re: [PR] HIVE-27760: Filter on date type partitioning column producing 0 results [hive]
Posted by "zabetak (via GitHub)" <gi...@apache.org>.
zabetak commented on code in PR #4760:
URL: https://github.com/apache/hive/pull/4760#discussion_r1347003115
##########
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/utils/TestMetaStoreUtils.java:
##########
@@ -0,0 +1,78 @@
+/*
+ * 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
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * 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.utils;
+
+import org.apache.hadoop.hive.metastore.annotation.MetastoreUnitTest;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import java.sql.Date;
+import java.sql.Timestamp;
+import java.time.ZoneId;
+import java.util.TimeZone;
+
+import static org.junit.Assert.assertEquals;
+
+@Category(MetastoreUnitTest.class)
+public class TestMetaStoreUtils {
+
+ @Test
+ public void testConvertDateToString() {
+ TimeZone defaultTZ = TimeZone.getDefault();
+ try {
+ TimeZone.setDefault(TimeZone.getTimeZone(ZoneId.of("Asia/Hong_Kong")));
+ String date = MetaStoreUtils.convertDateToString(Date.valueOf("2023-01-01"));
+ assertEquals("2023-01-01", date);
+ } finally {
+ TimeZone.setDefault(defaultTZ);
+ }
+ }
+
+ @Test
+ public void testcConvertTimestampToString() {
+ TimeZone defaultTZ = TimeZone.getDefault();
+ try {
+ String date = MetaStoreUtils.convertTimestampToString(Timestamp.valueOf("2023-01-01 10:20:30"));
+ assertEquals("2023-01-01 10:20:30", date);
+ } finally {
+ TimeZone.setDefault(defaultTZ);
+ }
Review Comment:
Yeap I agree its not a problem introduced by the PR but it is a bug that is probably due to DST shift. We have many of those in Hive :) Please log a new JIRA ticket and we can follow-up there.
--
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
Re: [PR] HIVE-27760: Filter on date type partitioning column producing 0 results [hive]
Posted by "zabetak (via GitHub)" <gi...@apache.org>.
zabetak commented on code in PR #4760:
URL: https://github.com/apache/hive/pull/4760#discussion_r1346998917
##########
ql/src/test/results/clientpositive/llap/date_timestamp_partition_filter.q.out:
##########
@@ -0,0 +1,214 @@
+PREHOOK: query: CREATE EXTERNAL TABLE testpd(col1 string, col2 String) PARTITIONED BY(PartitionDate DATE) STORED AS ORC
Review Comment:
The query result is definitely useful, I am asking for the EXPLAIN output.
--
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
Re: [PR] HIVE-27760: Filter on date type partitioning column producing 0 results [hive]
Posted by "mdayakar (via GitHub)" <gi...@apache.org>.
mdayakar commented on code in PR #4760:
URL: https://github.com/apache/hive/pull/4760#discussion_r1344101012
##########
ql/src/test/results/clientpositive/llap/date_timestamp_partition_filter.q.out:
##########
@@ -0,0 +1,214 @@
+PREHOOK: query: CREATE EXTERNAL TABLE testpd(col1 string, col2 String) PARTITIONED BY(PartitionDate DATE) STORED AS ORC
Review Comment:
https://github.com/apache/hive/pull/4760/files#diff-a03353b02b81f6c2c36cc130d0d85b019fb725c7cb0d57d0baa9b7122177610eR41 and https://github.com/apache/hive/pull/4760/files#diff-a03353b02b81f6c2c36cc130d0d85b019fb725c7cb0d57d0baa9b7122177610eR90
These two lines shows that partition pruning is happening.
--
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
Re: [PR] HIVE-27760: Filter on date type partitioning column producing 0 results [hive]
Posted by "mdayakar (via GitHub)" <gi...@apache.org>.
mdayakar commented on code in PR #4760:
URL: https://github.com/apache/hive/pull/4760#discussion_r1347346270
##########
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/utils/TestMetaStoreUtils.java:
##########
@@ -0,0 +1,78 @@
+/*
+ * 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
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * 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.utils;
+
+import org.apache.hadoop.hive.metastore.annotation.MetastoreUnitTest;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import java.sql.Date;
+import java.sql.Timestamp;
+import java.time.ZoneId;
+import java.util.TimeZone;
+
+import static org.junit.Assert.assertEquals;
+
+@Category(MetastoreUnitTest.class)
+public class TestMetaStoreUtils {
+
+ @Test
+ public void testConvertDateToString() {
+ TimeZone defaultTZ = TimeZone.getDefault();
+ try {
+ TimeZone.setDefault(TimeZone.getTimeZone(ZoneId.of("Asia/Hong_Kong")));
+ String date = MetaStoreUtils.convertDateToString(Date.valueOf("2023-01-01"));
+ assertEquals("2023-01-01", date);
+ } finally {
+ TimeZone.setDefault(defaultTZ);
+ }
+ }
+
+ @Test
+ public void testcConvertTimestampToString() {
+ TimeZone defaultTZ = TimeZone.getDefault();
+ try {
+ String date = MetaStoreUtils.convertTimestampToString(Timestamp.valueOf("2023-01-01 10:20:30"));
+ assertEquals("2023-01-01 10:20:30", date);
+ } finally {
+ TimeZone.setDefault(defaultTZ);
+ }
Review Comment:
Issue is observed for the timestamps from `9743-03-31 03:00:00` to `9743-03-31 03:59:59`.
--
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
Re: [PR] HIVE-27760: Filter on date type partitioning column producing 0 results [hive]
Posted by "mdayakar (via GitHub)" <gi...@apache.org>.
mdayakar commented on code in PR #4760:
URL: https://github.com/apache/hive/pull/4760#discussion_r1343811252
##########
ql/src/test/queries/clientpositive/date_timestamp_partition_filter.q:
##########
@@ -0,0 +1,14 @@
+--! qt:timezone:Asia/Hong_Kong
+
+CREATE EXTERNAL TABLE testpd(col1 string, col2 String) PARTITIONED BY(PartitionDate DATE) STORED AS ORC;
+INSERT into testpd(PartitionDate, col1, col2) VALUES('2023-01-01','Value11','Value12');
+INSERT into testpd(PartitionDate, col1, col2) VALUES('2023-01-02','Value21','Value22');
+explain extended select * from testpd where PartitionDate = '2023-01-01';
+select * from testpd where PartitionDate = '2023-01-01';
+
+
+CREATE EXTERNAL TABLE testpt(col1 string, col2 String) PARTITIONED BY(PartitionTimestamp TIMESTAMP) STORED AS ORC;
+INSERT into testpt(PartitionTimestamp, col1, col2) VALUES('2023-01-01 10:20:30','Value11','Value12');
+INSERT into testpt(PartitionTimestamp, col1, col2) VALUES('2023-01-02 20:30:40','Value21','Value22');
+explain extended select * from testpt where PartitionTimestamp = '2023-01-01 10:20:30';
+select * from testpt where PartitionTimestamp = '2023-01-01 10:20:30';
Review Comment:
Added new line.
--
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
Re: [PR] HIVE-27760: Filter on date type partitioning column producing 0 results [hive]
Posted by "zabetak (via GitHub)" <gi...@apache.org>.
zabetak commented on code in PR #4760:
URL: https://github.com/apache/hive/pull/4760#discussion_r1348668296
##########
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/utils/TestMetaStoreUtils.java:
##########
@@ -0,0 +1,78 @@
+/*
+ * 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
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * 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.utils;
+
+import org.apache.hadoop.hive.metastore.annotation.MetastoreUnitTest;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import java.sql.Date;
+import java.sql.Timestamp;
+import java.time.ZoneId;
+import java.util.TimeZone;
+
+import static org.junit.Assert.assertEquals;
+
+@Category(MetastoreUnitTest.class)
+public class TestMetaStoreUtils {
+
+ @Test
+ public void testConvertDateToString() {
+ TimeZone defaultTZ = TimeZone.getDefault();
+ try {
+ TimeZone.setDefault(TimeZone.getTimeZone(ZoneId.of("Asia/Hong_Kong")));
+ String date = MetaStoreUtils.convertDateToString(Date.valueOf("2023-01-01"));
+ assertEquals("2023-01-01", date);
+ } finally {
+ TimeZone.setDefault(defaultTZ);
+ }
+ }
+
+ @Test
+ public void testcConvertTimestampToString() {
+ TimeZone defaultTZ = TimeZone.getDefault();
+ try {
+ String date = MetaStoreUtils.convertTimestampToString(Timestamp.valueOf("2023-01-01 10:20:30"));
+ assertEquals("2023-01-01 10:20:30", date);
+ } finally {
+ TimeZone.setDefault(defaultTZ);
+ }
Review Comment:
https://issues.apache.org/jira/browse/HIVE-27775 is the ticket for the problem reported here.
--
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
Re: [PR] HIVE-27760: Filter on date type partitioning column producing 0 results [hive]
Posted by "mdayakar (via GitHub)" <gi...@apache.org>.
mdayakar commented on code in PR #4760:
URL: https://github.com/apache/hive/pull/4760#discussion_r1345341288
##########
ql/src/test/results/clientpositive/llap/date_timestamp_partition_filter.q.out:
##########
@@ -0,0 +1,214 @@
+PREHOOK: query: CREATE EXTERNAL TABLE testpd(col1 string, col2 String) PARTITIONED BY(PartitionDate DATE) STORED AS ORC
Review Comment:
https://github.com/apache/hive/pull/4760/files#diff-a03353b02b81f6c2c36cc130d0d85b019fb725c7cb0d57d0baa9b7122177610eR41 and https://github.com/apache/hive/pull/4760/files#diff-a03353b02b81f6c2c36cc130d0d85b019fb725c7cb0d57d0baa9b7122177610eR90 shows partition pruning is happening but this is at storage level.
--
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
Re: [PR] HIVE-27760: Filter on date type partitioning column producing 0 results [hive]
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #4760:
URL: https://github.com/apache/hive/pull/4760#issuecomment-1744702549
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=4760)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4760&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4760&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4760&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4760&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4760&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4760&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4760&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4760&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4760&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4760&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4760&resolved=false&types=CODE_SMELL) [3 Code Smells](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4760&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=4760&metric=coverage&view=list) No Coverage information
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=4760&metric=duplicated_lines_density&view=list) No Duplication information
 The version of Java (11.0.8) you have used to run this analysis is deprecated and we will stop accepting it soon. Please update to at least Java 17.
Read more [here](https://docs.sonarcloud.io/appendices/scanner-environment/)
--
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
Re: [PR] HIVE-27760: Filter on date type partitioning column producing 0 results [hive]
Posted by "mdayakar (via GitHub)" <gi...@apache.org>.
mdayakar commented on code in PR #4760:
URL: https://github.com/apache/hive/pull/4760#discussion_r1343810888
##########
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/utils/TestMetaStoreUtils.java:
##########
@@ -0,0 +1,78 @@
+/*
+ * 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
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * 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.utils;
+
+import org.apache.hadoop.hive.metastore.annotation.MetastoreUnitTest;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import java.sql.Date;
+import java.sql.Timestamp;
+import java.time.ZoneId;
+import java.util.TimeZone;
+
+import static org.junit.Assert.assertEquals;
+
+@Category(MetastoreUnitTest.class)
+public class TestMetaStoreUtils {
+
+ @Test
+ public void testConvertDateToString() {
+ TimeZone defaultTZ = TimeZone.getDefault();
+ try {
+ TimeZone.setDefault(TimeZone.getTimeZone(ZoneId.of("Asia/Hong_Kong")));
+ String date = MetaStoreUtils.convertDateToString(Date.valueOf("2023-01-01"));
+ assertEquals("2023-01-01", date);
+ } finally {
+ TimeZone.setDefault(defaultTZ);
+ }
+ }
+
+ @Test
+ public void testcConvertTimestampToString() {
+ TimeZone defaultTZ = TimeZone.getDefault();
+ try {
+ String date = MetaStoreUtils.convertTimestampToString(Timestamp.valueOf("2023-01-01 10:20:30"));
+ assertEquals("2023-01-01 10:20:30", date);
+ } finally {
+ TimeZone.setDefault(defaultTZ);
+ }
Review Comment:
Actually here I missed out `TimeZone.setDefault(TimeZone.getTimeZone(ZoneId.of("Asia/Hong_Kong")));` line in the try block. Thanks for pointing it out, now added in all the testcases.
--
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 #4760: HIVE-27760: Filter on date type partitioning column producing 0 results
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #4760:
URL: https://github.com/apache/hive/pull/4760#issuecomment-1741681615
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=4760)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4760&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4760&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4760&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4760&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4760&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4760&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4760&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4760&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4760&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4760&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4760&resolved=false&types=CODE_SMELL) [3 Code Smells](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4760&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=4760&metric=coverage&view=list) No Coverage information
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=4760&metric=duplicated_lines_density&view=list) No Duplication information
 The version of Java (11.0.8) you have used to run this analysis is deprecated and we will stop accepting it soon. Please update to at least Java 17.
Read more [here](https://docs.sonarcloud.io/appendices/scanner-environment/)
--
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
Re: [PR] HIVE-27760: Filter on date type partitioning column producing 0 results [hive]
Posted by "mdayakar (via GitHub)" <gi...@apache.org>.
mdayakar commented on code in PR #4760:
URL: https://github.com/apache/hive/pull/4760#discussion_r1344032714
##########
ql/src/java/org/apache/hadoop/hive/ql/parse/BaseSemanticAnalyzer.java:
##########
@@ -1804,9 +1803,9 @@ private static String normalizeDateCol(Object colValue) throws SemanticException
throw new SemanticException("Unexpected date type " + colValue.getClass());
}
try {
- return MetaStoreUtils.PARTITION_DATE_FORMAT.get().format(
- MetaStoreUtils.PARTITION_DATE_FORMAT.get().parse(value.toString()));
- } catch (ParseException e) {
+ return MetaStoreUtils.convertDateToString(
+ MetaStoreUtils.convertStringToDate(value.toString()));
+ } catch (Exception e) {
throw new SemanticException(e);
}
}
Review Comment:
Yeah, I could see only testcode is calling it. If required we can remove it(anyway as you mentioned its not related to the current 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: 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
Re: [PR] HIVE-27760: Filter on date type partitioning column producing 0 results [hive]
Posted by "zabetak (via GitHub)" <gi...@apache.org>.
zabetak commented on code in PR #4760:
URL: https://github.com/apache/hive/pull/4760#discussion_r1345783131
##########
ql/src/test/results/clientpositive/llap/date_timestamp_partition_filter.q.out:
##########
@@ -0,0 +1,214 @@
+PREHOOK: query: CREATE EXTERNAL TABLE testpd(col1 string, col2 String) PARTITIONED BY(PartitionDate DATE) STORED AS ORC
Review Comment:
I will ask it differently. Is there anything in the EXPLAIN or elsewhere in the .q.out file that proves that we passed from the buggy code? If the EXPLAIN is the same with and without the changes here then it seems redundant.
##########
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/utils/TestMetaStoreUtils.java:
##########
@@ -0,0 +1,78 @@
+/*
+ * 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
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * 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.utils;
+
+import org.apache.hadoop.hive.metastore.annotation.MetastoreUnitTest;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import java.sql.Date;
+import java.sql.Timestamp;
+import java.time.ZoneId;
+import java.util.TimeZone;
+
+import static org.junit.Assert.assertEquals;
+
+@Category(MetastoreUnitTest.class)
+public class TestMetaStoreUtils {
+
+ @Test
+ public void testConvertDateToString() {
+ TimeZone defaultTZ = TimeZone.getDefault();
+ try {
+ TimeZone.setDefault(TimeZone.getTimeZone(ZoneId.of("Asia/Hong_Kong")));
+ String date = MetaStoreUtils.convertDateToString(Date.valueOf("2023-01-01"));
+ assertEquals("2023-01-01", date);
+ } finally {
+ TimeZone.setDefault(defaultTZ);
+ }
+ }
+
+ @Test
+ public void testcConvertTimestampToString() {
+ TimeZone defaultTZ = TimeZone.getDefault();
+ try {
+ String date = MetaStoreUtils.convertTimestampToString(Timestamp.valueOf("2023-01-01 10:20:30"));
+ assertEquals("2023-01-01 10:20:30", date);
+ } finally {
+ TimeZone.setDefault(defaultTZ);
+ }
Review Comment:
I pushed a commit (https://github.com/apache/hive/pull/4760/commits/4ff9ba968b31ea16611ba5d5bc05bbfb3b4fe184) based on what I had in mind. Have a look and let me know if I missed something.
With the new tests, I found a bug in `convertTimestampToString` using [zoneId=Europe/Paris, timestamp=9743-03-31T03:37:33] but I think this existed since day one so we can possibly defer it to another 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: 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
Re: [PR] HIVE-27760: Filter on date type partitioning column producing 0 results [hive]
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #4760:
URL: https://github.com/apache/hive/pull/4760#issuecomment-1750009420
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=4760)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4760&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4760&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4760&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4760&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4760&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4760&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4760&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4760&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4760&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4760&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4760&resolved=false&types=CODE_SMELL) [3 Code Smells](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4760&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=4760&metric=coverage&view=list) No Coverage information
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=4760&metric=duplicated_lines_density&view=list) No Duplication information
 The version of Java (11.0.8) you have used to run this analysis is deprecated and we will stop accepting it soon. Please update to at least Java 17.
Read more [here](https://docs.sonarcloud.io/appendices/scanner-environment/)
--
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
Re: [PR] HIVE-27760: Filter on date type partitioning column producing 0 results [hive]
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #4760:
URL: https://github.com/apache/hive/pull/4760#issuecomment-1747044424
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=4760)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4760&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4760&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4760&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4760&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4760&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4760&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4760&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4760&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4760&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4760&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4760&resolved=false&types=CODE_SMELL) [3 Code Smells](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4760&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=4760&metric=coverage&view=list) No Coverage information
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=4760&metric=duplicated_lines_density&view=list) No Duplication information
 The version of Java (11.0.8) you have used to run this analysis is deprecated and we will stop accepting it soon. Please update to at least Java 17.
Read more [here](https://docs.sonarcloud.io/appendices/scanner-environment/)
--
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 #4760: HIVE-27760: Filter on date type partitioning column producing 0 results
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #4760:
URL: https://github.com/apache/hive/pull/4760#issuecomment-1741347758
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=4760)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4760&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4760&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4760&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4760&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4760&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4760&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4760&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4760&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4760&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4760&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4760&resolved=false&types=CODE_SMELL) [10 Code Smells](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4760&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=4760&metric=coverage&view=list) No Coverage information
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=4760&metric=duplicated_lines_density&view=list) No Duplication information
 The version of Java (11.0.8) you have used to run this analysis is deprecated and we will stop accepting it soon. Please update to at least Java 17.
Read more [here](https://docs.sonarcloud.io/appendices/scanner-environment/)
--
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 #4760: HIVE-27760: Filter on date type partitioning column producing 0 results
Posted by "zabetak (via GitHub)" <gi...@apache.org>.
zabetak commented on code in PR #4760:
URL: https://github.com/apache/hive/pull/4760#discussion_r1341952353
##########
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/utils/MetaStoreUtils.java:
##########
@@ -74,25 +77,40 @@
import com.google.common.base.Joiner;
public class MetaStoreUtils {
- /** A fixed date format to be used for hive partition column values. */
- public static final ThreadLocal<DateFormat> PARTITION_DATE_FORMAT =
- new ThreadLocal<DateFormat>() {
- @Override
- protected DateFormat initialValue() {
- DateFormat 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;
- }
- };
- public static final ThreadLocal<DateTimeFormatter> PARTITION_TIMESTAMP_FORMAT =
- new ThreadLocal<DateTimeFormatter>() {
- @Override
- protected DateTimeFormatter initialValue() {
- return DateTimeFormatter.ofPattern("yyyy-MM-dd HH:mm:ss").
- withZone(TimeZone.getTimeZone("UTC").toZoneId());
- }
- };
+
+ private static final DateTimeFormatter DATE_FORMATTER = createDateTimeFormatter("uuuu-MM-dd");
+
+ private static final DateTimeFormatter TIMESTAMP_FORMATTER = createDateTimeFormatter("uuuu-MM-dd HH:mm:ss");
+
+ private static DateTimeFormatter createDateTimeFormatter(String format) {
+ return DateTimeFormatter.ofPattern(format)
+ .withZone(TimeZone.getTimeZone("UTC").toZoneId())
+ .withResolverStyle(ResolverStyle.STRICT);
+ }
+
+ /**
+ * Converts java.sql.Date to String format.
+ * @param date - java.sql.Date object.
+ * @return Date in string format.
+ */
+ public static String convertDateToString(Date date) {
+ return DATE_FORMATTER.format(date.toLocalDate());
+ }
+
+ public static Date convertStringToDate(String date) {
+ LocalDate val = LocalDate.parse(date, DATE_FORMATTER);
+ return java.sql.Date.valueOf(val);
+ }
+
+ public static String convertTimestampToString(Timestamp timestamp) {
+ return TIMESTAMP_FORMATTER.format(timestamp.toLocalDateTime());
+ }
+
+ public static Timestamp convertStringToTimestamp(String timestamp) {
+ LocalDateTime val = LocalDateTime.from(TIMESTAMP_FORMATTER.parse(timestamp));
+ return Timestamp.valueOf(val);
+ }
+
Review Comment:
To avoid any kind of unpleasant surprises we should use the same methods and formatters in `PartFilterVisitor`.
##########
ql/src/test/results/clientpositive/llap/date_timestamp_partition_filter.q.out:
##########
@@ -0,0 +1,214 @@
+PREHOOK: query: CREATE EXTERNAL TABLE testpd(col1 string, col2 String) PARTITIONED BY(PartitionDate DATE) STORED AS ORC
Review Comment:
Is there a part in this output that shows that partition pruning is working and there are no hidden exceptions that just prevent it from running?
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/parser/ExpressionTree.java:
##########
@@ -18,7 +18,7 @@
package org.apache.hadoop.hive.metastore.parser;
import java.sql.Timestamp;
-import java.util.Date;
Review Comment:
That's a sneaky change since `java.util.Date` is a super class of `java.sql.Date` so the behavior of `instanceof` and other operators may be affected.
Do we have test coverage for this class? Are we creating anywhere else instances of `java.util.Date` that may end up here?
##########
ql/src/java/org/apache/hadoop/hive/ql/parse/BaseSemanticAnalyzer.java:
##########
@@ -1804,9 +1803,9 @@ private static String normalizeDateCol(Object colValue) throws SemanticException
throw new SemanticException("Unexpected date type " + colValue.getClass());
}
try {
- return MetaStoreUtils.PARTITION_DATE_FORMAT.get().format(
- MetaStoreUtils.PARTITION_DATE_FORMAT.get().parse(value.toString()));
- } catch (ParseException e) {
+ return MetaStoreUtils.convertDateToString(
+ MetaStoreUtils.convertStringToDate(value.toString()));
+ } catch (Exception e) {
throw new SemanticException(e);
}
}
Review Comment:
Interestingly this seems to be dead code. We probably don't need to do something as part of this PR but I was wondering why it was left here. We can investigate later on after we are done with this.
##########
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/utils/TestMetaStoreUtils.java:
##########
@@ -0,0 +1,78 @@
+/*
+ * 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
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * 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.utils;
+
+import org.apache.hadoop.hive.metastore.annotation.MetastoreUnitTest;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import java.sql.Date;
+import java.sql.Timestamp;
+import java.time.ZoneId;
+import java.util.TimeZone;
+
+import static org.junit.Assert.assertEquals;
+
+@Category(MetastoreUnitTest.class)
+public class TestMetaStoreUtils {
+
+ @Test
+ public void testConvertDateToString() {
+ TimeZone defaultTZ = TimeZone.getDefault();
+ try {
+ TimeZone.setDefault(TimeZone.getTimeZone(ZoneId.of("Asia/Hong_Kong")));
+ String date = MetaStoreUtils.convertDateToString(Date.valueOf("2023-01-01"));
+ assertEquals("2023-01-01", date);
+ } finally {
+ TimeZone.setDefault(defaultTZ);
+ }
+ }
+
+ @Test
+ public void testcConvertTimestampToString() {
+ TimeZone defaultTZ = TimeZone.getDefault();
+ try {
+ String date = MetaStoreUtils.convertTimestampToString(Timestamp.valueOf("2023-01-01 10:20:30"));
+ assertEquals("2023-01-01 10:20:30", date);
+ } finally {
+ TimeZone.setDefault(defaultTZ);
+ }
Review Comment:
Why do we need this? Neither the test nor the called method modify the default timezone.
##########
ql/src/test/queries/clientpositive/date_timestamp_partition_filter.q:
##########
@@ -0,0 +1,14 @@
+--! qt:timezone:Asia/Hong_Kong
+
+CREATE EXTERNAL TABLE testpd(col1 string, col2 String) PARTITIONED BY(PartitionDate DATE) STORED AS ORC;
+INSERT into testpd(PartitionDate, col1, col2) VALUES('2023-01-01','Value11','Value12');
+INSERT into testpd(PartitionDate, col1, col2) VALUES('2023-01-02','Value21','Value22');
+explain extended select * from testpd where PartitionDate = '2023-01-01';
+select * from testpd where PartitionDate = '2023-01-01';
+
+
+CREATE EXTERNAL TABLE testpt(col1 string, col2 String) PARTITIONED BY(PartitionTimestamp TIMESTAMP) STORED AS ORC;
+INSERT into testpt(PartitionTimestamp, col1, col2) VALUES('2023-01-01 10:20:30','Value11','Value12');
+INSERT into testpt(PartitionTimestamp, col1, col2) VALUES('2023-01-02 20:30:40','Value21','Value22');
+explain extended select * from testpt where PartitionTimestamp = '2023-01-01 10:20:30';
+select * from testpt where PartitionTimestamp = '2023-01-01 10:20:30';
Review Comment:
nit: Missing new line at the EOF.
##########
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/utils/TestMetaStoreUtils.java:
##########
@@ -0,0 +1,78 @@
+/*
+ * 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
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * 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.utils;
+
+import org.apache.hadoop.hive.metastore.annotation.MetastoreUnitTest;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import java.sql.Date;
+import java.sql.Timestamp;
+import java.time.ZoneId;
+import java.util.TimeZone;
+
+import static org.junit.Assert.assertEquals;
+
+@Category(MetastoreUnitTest.class)
+public class TestMetaStoreUtils {
Review Comment:
Would some of these tests fail without the new changes introduced in this PR?
##########
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/utils/TestMetaStoreUtils.java:
##########
@@ -0,0 +1,78 @@
+/*
+ * 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
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * 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.utils;
+
+import org.apache.hadoop.hive.metastore.annotation.MetastoreUnitTest;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import java.sql.Date;
+import java.sql.Timestamp;
+import java.time.ZoneId;
+import java.util.TimeZone;
+
+import static org.junit.Assert.assertEquals;
+
+@Category(MetastoreUnitTest.class)
+public class TestMetaStoreUtils {
Review Comment:
Can we also add tests for:
* String -> Date -> String
* String -> Timestamp -> String
--
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
Re: [PR] HIVE-27760: Filter on date type partitioning column producing 0 results [hive]
Posted by "mdayakar (via GitHub)" <gi...@apache.org>.
mdayakar commented on code in PR #4760:
URL: https://github.com/apache/hive/pull/4760#discussion_r1345338996
##########
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/utils/TestMetaStoreUtils.java:
##########
@@ -0,0 +1,78 @@
+/*
+ * 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
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * 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.utils;
+
+import org.apache.hadoop.hive.metastore.annotation.MetastoreUnitTest;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import java.sql.Date;
+import java.sql.Timestamp;
+import java.time.ZoneId;
+import java.util.TimeZone;
+
+import static org.junit.Assert.assertEquals;
+
+@Category(MetastoreUnitTest.class)
+public class TestMetaStoreUtils {
+
+ @Test
+ public void testConvertDateToString() {
+ TimeZone defaultTZ = TimeZone.getDefault();
+ try {
+ TimeZone.setDefault(TimeZone.getTimeZone(ZoneId.of("Asia/Hong_Kong")));
+ String date = MetaStoreUtils.convertDateToString(Date.valueOf("2023-01-01"));
+ assertEquals("2023-01-01", date);
+ } finally {
+ TimeZone.setDefault(defaultTZ);
+ }
+ }
+
+ @Test
+ public void testcConvertTimestampToString() {
+ TimeZone defaultTZ = TimeZone.getDefault();
+ try {
+ String date = MetaStoreUtils.convertTimestampToString(Timestamp.valueOf("2023-01-01 10:20:30"));
+ assertEquals("2023-01-01 10:20:30", date);
+ } finally {
+ TimeZone.setDefault(defaultTZ);
+ }
Review Comment:
Updated the testcases accordingly but I couldn't use `@Parameterized` due to some maven dependency issues.
--
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
Re: [PR] HIVE-27760: Filter on date type partitioning column producing 0 results [hive]
Posted by "mdayakar (via GitHub)" <gi...@apache.org>.
mdayakar commented on code in PR #4760:
URL: https://github.com/apache/hive/pull/4760#discussion_r1343797487
##########
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/utils/MetaStoreUtils.java:
##########
@@ -74,25 +77,40 @@
import com.google.common.base.Joiner;
public class MetaStoreUtils {
- /** A fixed date format to be used for hive partition column values. */
- public static final ThreadLocal<DateFormat> PARTITION_DATE_FORMAT =
- new ThreadLocal<DateFormat>() {
- @Override
- protected DateFormat initialValue() {
- DateFormat 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;
- }
- };
- public static final ThreadLocal<DateTimeFormatter> PARTITION_TIMESTAMP_FORMAT =
- new ThreadLocal<DateTimeFormatter>() {
- @Override
- protected DateTimeFormatter initialValue() {
- return DateTimeFormatter.ofPattern("yyyy-MM-dd HH:mm:ss").
- withZone(TimeZone.getTimeZone("UTC").toZoneId());
- }
- };
+
+ private static final DateTimeFormatter DATE_FORMATTER = createDateTimeFormatter("uuuu-MM-dd");
+
+ private static final DateTimeFormatter TIMESTAMP_FORMATTER = createDateTimeFormatter("uuuu-MM-dd HH:mm:ss");
+
+ private static DateTimeFormatter createDateTimeFormatter(String format) {
+ return DateTimeFormatter.ofPattern(format)
+ .withZone(TimeZone.getTimeZone("UTC").toZoneId())
+ .withResolverStyle(ResolverStyle.STRICT);
+ }
+
+ /**
+ * Converts java.sql.Date to String format.
+ * @param date - java.sql.Date object.
+ * @return Date in string format.
+ */
+ public static String convertDateToString(Date date) {
+ return DATE_FORMATTER.format(date.toLocalDate());
+ }
+
+ public static Date convertStringToDate(String date) {
+ LocalDate val = LocalDate.parse(date, DATE_FORMATTER);
+ return java.sql.Date.valueOf(val);
+ }
+
+ public static String convertTimestampToString(Timestamp timestamp) {
+ return TIMESTAMP_FORMATTER.format(timestamp.toLocalDateTime());
+ }
+
+ public static Timestamp convertStringToTimestamp(String timestamp) {
+ LocalDateTime val = LocalDateTime.from(TIMESTAMP_FORMATTER.parse(timestamp));
+ return Timestamp.valueOf(val);
+ }
+
Review Comment:
Modified `PartFilterVisitor` to use `MetaStoreUtils` APIs. Now both `PartFilterVisitor` and `MetaStoreDirectSql` are using same APIs.
--
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
Re: [PR] HIVE-27760: Filter on date type partitioning column producing 0 results [hive]
Posted by "mdayakar (via GitHub)" <gi...@apache.org>.
mdayakar commented on code in PR #4760:
URL: https://github.com/apache/hive/pull/4760#discussion_r1343805984
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/parser/ExpressionTree.java:
##########
@@ -18,7 +18,7 @@
package org.apache.hadoop.hive.metastore.parser;
import java.sql.Timestamp;
-import java.util.Date;
Review Comment:
Actually in the flow `java.sql.Date `object only coming here, initially when added it was added wrongly as `java.util.Date`. As `java.util.Date` is super class for `java.sql.Date`, `value instanceof java.util.Date` is working fine till now. `partition_date.q` test is covering 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: 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
Re: [PR] HIVE-27760: Filter on date type partitioning column producing 0 results [hive]
Posted by "mdayakar (via GitHub)" <gi...@apache.org>.
mdayakar commented on code in PR #4760:
URL: https://github.com/apache/hive/pull/4760#discussion_r1344101012
##########
ql/src/test/results/clientpositive/llap/date_timestamp_partition_filter.q.out:
##########
@@ -0,0 +1,214 @@
+PREHOOK: query: CREATE EXTERNAL TABLE testpd(col1 string, col2 String) PARTITIONED BY(PartitionDate DATE) STORED AS ORC
Review Comment:
https://github.com/apache/hive/pull/4760/files#diff-a03353b02b81f6c2c36cc130d0d85b019fb725c7cb0d57d0baa9b7122177610eR41 and https://github.com/apache/hive/pull/4760/files#diff-a03353b02b81f6c2c36cc130d0d85b019fb725c7cb0d57d0baa9b7122177610eR90
These two lines shows that partition pruning is happening.
--
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