You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@carbondata.apache.org by mohammadshahidkhan <gi...@git.apache.org> on 2017/07/03 10:34:54 UTC
[GitHub] carbondata pull request #1126: [CARBONDATA-1258] CarbonData should not allow...
GitHub user mohammadshahidkhan opened a pull request:
https://github.com/apache/carbondata/pull/1126
[CARBONDATA-1258] CarbonData should not allow loading Date Type value…
…s violating the boundary condition ("0001-01-01" through "9999-12-31")
For Date Type the value X being loaded must be X >= "0001-01-01" and X <= "9999-12-31".
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/mohammadshahidkhan/incubator-carbondata datecolumnissue
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/carbondata/pull/1126.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #1126
----
commit 3d442e6041582332dc26eb7cc56cb5366f4f96db
Author: mohammadshahidkhan <mo...@gmail.com>
Date: 2017-07-03T10:18:40Z
[CARBONDATA-1258] CarbonData should not allow loading Date Type values violating the boundary condition ("0001-01-01" through "9999-12-31")
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] carbondata issue #1126: [CARBONDATA-1258] CarbonData should not allow loadin...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/1126
Build Success with Spark 2.2.0, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/982/
---
[GitHub] carbondata issue #1126: [CARBONDATA-1258] CarbonData should not allow loadin...
Posted by mohammadshahidkhan <gi...@git.apache.org>.
Github user mohammadshahidkhan commented on the issue:
https://github.com/apache/carbondata/pull/1126
retest this please
---
[GitHub] carbondata pull request #1126: [CARBONDATA-1258] CarbonData should not allow...
Posted by mohammadshahidkhan <gi...@git.apache.org>.
Github user mohammadshahidkhan commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/1126#discussion_r133404888
--- Diff: core/src/main/java/org/apache/carbondata/core/keygenerator/directdictionary/timestamp/DateDirectDictionaryGenerator.java ---
@@ -154,6 +162,12 @@ private int generateDirectSurrogateKeyForNonTimestampType(String memberStr) {
}
private int generateKey(long timeValue) {
+ if (timeValue < MIN_VALUE || timeValue > MAX_VALUE) {
+ if (LOGGER.isDebugEnabled()) {
+ LOGGER.debug("Value for date type column is not in valid range. Value considered as null.");
+ }
+ return 1;
--- End diff --
if the value is not in defined range then it will be store as null, if bad records action is FORCE.
For null value dictionary key is 1.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] carbondata issue #1126: [CARBONDATA-1258] CarbonData should not allow loadin...
Posted by mohammadshahidkhan <gi...@git.apache.org>.
Github user mohammadshahidkhan commented on the issue:
https://github.com/apache/carbondata/pull/1126
retest this please
---
[GitHub] carbondata issue #1126: [CARBONDATA-1258] CarbonData should not allow loadin...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/1126
Build Success with Spark 2.2.0, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/881/
---
[GitHub] carbondata issue #1126: [CARBONDATA-1258] CarbonData should not allow loadin...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit commented on the issue:
https://github.com/apache/carbondata/pull/1126
Can one of the admins verify this patch?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] carbondata issue #1126: [CARBONDATA-1258] CarbonData should not allow loadin...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/1126
Build Success with Spark 2.2.0, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/895/
---
[GitHub] carbondata issue #1126: [CARBONDATA-1258] CarbonData should not allow loadin...
Posted by manishgupta88 <gi...@git.apache.org>.
Github user manishgupta88 commented on the issue:
https://github.com/apache/carbondata/pull/1126
retest this please
---
[GitHub] carbondata issue #1126: [CARBONDATA-1258] CarbonData should not allow loadin...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/1126
Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/41/
---
[GitHub] carbondata pull request #1126: [CARBONDATA-1258] CarbonData should not allow...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/carbondata/pull/1126
---
[GitHub] carbondata issue #1126: [CARBONDATA-1258] CarbonData should not allow loadin...
Posted by manishgupta88 <gi...@git.apache.org>.
Github user manishgupta88 commented on the issue:
https://github.com/apache/carbondata/pull/1126
Correct the PR header also...remove dots from the end and put a proper message
---
[GitHub] carbondata pull request #1126: [CARBONDATA-1258] CarbonData should not allow...
Posted by mohammadshahidkhan <gi...@git.apache.org>.
Github user mohammadshahidkhan commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/1126#discussion_r157729475
--- Diff: core/src/main/java/org/apache/carbondata/core/keygenerator/directdictionary/timestamp/DateDirectDictionaryGenerator.java ---
@@ -42,12 +42,37 @@
private String dateFormat;
+ /**
+ * min value supported for date type column
+ */
+ private static final long MIN_VALUE;
+ /**
+ * MAx value supported for date type column
+ */
+ private static final long MAX_VALUE;
/**
* Logger instance
*/
private static final LogService LOGGER =
LogServiceFactory.getLogService(DateDirectDictionaryGenerator.class.getName());
+ static {
+ SimpleDateFormat df = new SimpleDateFormat("yyyy-MM-dd");
+ df.setTimeZone(TimeZone.getTimeZone("GMT"));
+ long minValue = 0;
+ long maxValue = 0;
+ try {
+ minValue = df.parse("0001-01-01").getTime();
+ maxValue = df.parse("9999-12-31").getTime();
+ } catch (ParseException e) {
--- End diff --
Fixed
---
[GitHub] carbondata issue #1126: [CARBONDATA-1258] CarbonData should not allow loadin...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/1126
Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/587/
---
[GitHub] carbondata pull request #1126: [CARBONDATA-1258] CarbonData should not allow...
Posted by manishgupta88 <gi...@git.apache.org>.
Github user manishgupta88 commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/1126#discussion_r157700973
--- Diff: core/src/main/java/org/apache/carbondata/core/keygenerator/directdictionary/timestamp/DateDirectDictionaryGenerator.java ---
@@ -42,12 +42,37 @@
private String dateFormat;
+ /**
+ * min value supported for date type column
+ */
+ private static final long MIN_VALUE;
+ /**
+ * MAx value supported for date type column
+ */
+ private static final long MAX_VALUE;
/**
* Logger instance
*/
private static final LogService LOGGER =
LogServiceFactory.getLogService(DateDirectDictionaryGenerator.class.getName());
+ static {
+ SimpleDateFormat df = new SimpleDateFormat("yyyy-MM-dd");
+ df.setTimeZone(TimeZone.getTimeZone("GMT"));
+ long minValue = 0;
+ long maxValue = 0;
+ try {
+ minValue = df.parse("0001-01-01").getTime();
+ maxValue = df.parse("9999-12-31").getTime();
+ } catch (ParseException e) {
--- End diff --
As you are defining the date format yourself and parsing it, ParseException will not be thrown. But as we need to handle this exception, you can just put a warning logger in catch block and take out the remaining logic. If we dont get any findbug for empty catch block then better to just put a comment in catch block and not do anything
---
[GitHub] carbondata issue #1126: [CARBONDATA-1258] CarbonData should not allow loadin...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/1126
Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/2205/
---
[GitHub] carbondata issue #1126: [CARBONDATA-1258] CarbonData should not allow loadin...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/1126
Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder/2871/
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] carbondata issue #1126: [CARBONDATA-1258] CarbonData should not allow loadin...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/1126
Build Failed with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/2132/
---
[GitHub] carbondata issue #1126: [CARBONDATA-1258] CarbonData should not allow loadin...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/1126
Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/2120/
---
[GitHub] carbondata issue #1126: [CARBONDATA-1258] CarbonData should not allow loadin...
Posted by ravipesala <gi...@git.apache.org>.
Github user ravipesala commented on the issue:
https://github.com/apache/carbondata/pull/1126
SDV Build Fail , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/1218/
---
[GitHub] carbondata issue #1126: [CARBONDATA-1258] CarbonData should not allow loadin...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/1126
Can one of the admins verify this patch?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] carbondata issue #1126: [CARBONDATA-1258] CarbonData should not allow loadin...
Posted by ravipesala <gi...@git.apache.org>.
Github user ravipesala commented on the issue:
https://github.com/apache/carbondata/pull/1126
SDV Build Success , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/2393/
---
[GitHub] carbondata pull request #1126: [CARBONDATA-1258] CarbonData should not allow...
Posted by cenyuhai <gi...@git.apache.org>.
Github user cenyuhai commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/1126#discussion_r127450363
--- Diff: core/src/main/java/org/apache/carbondata/core/keygenerator/directdictionary/timestamp/DateDirectDictionaryGenerator.java ---
@@ -154,6 +162,12 @@ private int generateDirectSurrogateKeyForNonTimestampType(String memberStr) {
}
private int generateKey(long timeValue) {
+ if (timeValue < MIN_VALUE || timeValue > MAX_VALUE) {
+ if (LOGGER.isDebugEnabled()) {
+ LOGGER.debug("Value for date type column is not in valid range. Value considered as null.");
+ }
+ return 1;
--- End diff --
why return 1?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] carbondata issue #1126: [CARBONDATA-1258] CarbonData should not allow loadin...
Posted by manishgupta88 <gi...@git.apache.org>.
Github user manishgupta88 commented on the issue:
https://github.com/apache/carbondata/pull/1126
LGTM
---
[GitHub] carbondata issue #1126: [CARBONDATA-1258] CarbonData should not allow loadin...
Posted by ravipesala <gi...@git.apache.org>.
Github user ravipesala commented on the issue:
https://github.com/apache/carbondata/pull/1126
SDV Build Fail , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/2405/
---
[GitHub] carbondata issue #1126: [CARBONDATA-1258] CarbonData should not allow loadin...
Posted by mohammadshahidkhan <gi...@git.apache.org>.
Github user mohammadshahidkhan commented on the issue:
https://github.com/apache/carbondata/pull/1126
SDV failure is random, not related to this PR
---
[GitHub] carbondata issue #1126: [CARBONDATA-1258] CarbonData should not allow loadin...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/1126
Build Success with Spark 1.6, Please check CI http://144.76.159.231:8080/job/ApacheCarbonPRBuilder/288/
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] carbondata issue #1126: [CARBONDATA-1258] CarbonData should not allow loadin...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/1126
Build Failed with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/2106/
---
[GitHub] carbondata issue #1126: [CARBONDATA-1258] CarbonData should not allow loadin...
Posted by ravipesala <gi...@git.apache.org>.
Github user ravipesala commented on the issue:
https://github.com/apache/carbondata/pull/1126
SDV Build Fail , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/2414/
---