You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@carbondata.apache.org by kumarvishal09 <gi...@git.apache.org> on 2018/02/27 10:42:00 UTC
[GitHub] carbondata pull request #2004: [CARBONDATA-2208]Pre aggregate datamap creati...
GitHub user kumarvishal09 opened a pull request:
https://github.com/apache/carbondata/pull/2004
[CARBONDATA-2208]Pre aggregate datamap creation is failing when count(*) present in query
Pre aggregate data map creation is failing with parsing error
create datamap agg9 on table maintable using 'preaggregate' as select name, count from maintable group by name
your contribution quickly and easily:
- [ ] Any interfaces changed?
- [ ] Any backward compatibility impacted?
- [ ] Document update required?
- [ ] Testing done
Added UT
- [ ] For large changes, please consider breaking it into sub-tasks under an umbrella JIRA.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/kumarvishal09/incubator-carbondata master_26-02-2018
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/carbondata/pull/2004.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 #2004
----
commit 7b51a59148b570093fe82f3a14217c1c24886509
Author: kumarvishal <ku...@...>
Date: 2018-02-27T10:31:06Z
Fixed count(*) query issue in pre aggregate
----
---
[GitHub] carbondata issue #2004: [CARBONDATA-2208]Pre aggregate datamap creation is f...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/2004
Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/3940/
---
[GitHub] carbondata pull request #2004: [CARBONDATA-2208]Pre aggregate datamap creati...
Posted by xubo245 <gi...@git.apache.org>.
Github user xubo245 commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2004#discussion_r170906653
--- Diff: integration/spark2/src/main/scala/org/apache/spark/sql/execution/command/preaaggregate/PreAggregateUtil.scala ---
@@ -346,11 +346,13 @@ object PreAggregateUtil {
carbonTable)
}
// if parent column relation is of size more than one that means aggregate table
- // column is derived from multiple column of main table
- // or if expression is not a instance of attribute reference
+ // column is derived from multiple column of main table or if size is zero then it means
+ // column is present in select statement is some constants for example count(*)
+ // and if expression is not a instance of attribute reference
// then use column name which is passed
val columnName =
- if (parentColumnsName.size > 1 && !expression.isInstanceOf[AttributeReference]) {
+ if ((parentColumnsName.size > 1 || parentColumnsName.isEmpty) &&
+ !expression.isInstanceOf[AttributeReference]) {
newColumnName
--- End diff --
Why don't we use 1_count instead of 0_count in default?
It is count(1) after spark parser count(*)
---
[GitHub] carbondata pull request #2004: [CARBONDATA-2208]Pre aggregate datamap creati...
Posted by kumarvishal09 <gi...@git.apache.org>.
Github user kumarvishal09 commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2004#discussion_r170908712
--- Diff: integration/spark2/src/main/scala/org/apache/spark/sql/execution/command/preaaggregate/PreAggregateUtil.scala ---
@@ -346,11 +346,13 @@ object PreAggregateUtil {
carbonTable)
}
// if parent column relation is of size more than one that means aggregate table
- // column is derived from multiple column of main table
- // or if expression is not a instance of attribute reference
+ // column is derived from multiple column of main table or if size is zero then it means
+ // column is present in select statement is some constants for example count(*)
+ // and if expression is not a instance of attribute reference
// then use column name which is passed
val columnName =
- if (parentColumnsName.size > 1 && !expression.isInstanceOf[AttributeReference]) {
+ if ((parentColumnsName.size > 1 || parentColumnsName.isEmpty) &&
+ !expression.isInstanceOf[AttributeReference]) {
newColumnName
--- End diff --
i didn't get your comment
---
[GitHub] carbondata pull request #2004: [CARBONDATA-2208]Pre aggregate datamap creati...
Posted by kumarvishal09 <gi...@git.apache.org>.
Github user kumarvishal09 commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2004#discussion_r170916613
--- Diff: core/src/main/java/org/apache/carbondata/core/util/path/CarbonTablePath.java ---
@@ -75,7 +75,12 @@ public CarbonTablePath(CarbonTableIdentifier carbonTableIdentifier, String table
* @param carbonFilePath
*/
public static String getFolderContainingFile(String carbonFilePath) {
- return carbonFilePath.substring(0, carbonFilePath.lastIndexOf('/'));
+ int lastIndex = carbonFilePath.lastIndexOf('/');
+ // below code for handling windows environment
+ if (-1 == lastIndex) {
--- End diff --
E:/xubo\idea will never come either it will be E:/xubo/idea or E:\xubo\idea ?
---
[GitHub] carbondata issue #2004: [CARBONDATA-2208]Pre aggregate datamap creation is f...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/2004
Build Success with Spark 2.2.1, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/2696/
---
[GitHub] carbondata pull request #2004: [CARBONDATA-2208]Pre aggregate datamap creati...
Posted by kumarvishal09 <gi...@git.apache.org>.
Github user kumarvishal09 commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2004#discussion_r170909437
--- Diff: core/src/main/java/org/apache/carbondata/core/util/path/CarbonTablePath.java ---
@@ -75,7 +75,12 @@ public CarbonTablePath(CarbonTableIdentifier carbonTableIdentifier, String table
* @param carbonFilePath
*/
public static String getFolderContainingFile(String carbonFilePath) {
- return carbonFilePath.substring(0, carbonFilePath.lastIndexOf('/'));
+ int lastIndex = carbonFilePath.lastIndexOf('/');
+ // below code for handling windows environment
+ if (-1 == lastIndex) {
--- End diff --
in windows file seperator is "\\" , this is to handle the same
---
[GitHub] carbondata pull request #2004: [CARBONDATA-2208]Pre aggregate datamap creati...
Posted by xubo245 <gi...@git.apache.org>.
Github user xubo245 commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2004#discussion_r170922780
--- Diff: core/src/main/java/org/apache/carbondata/core/util/path/CarbonTablePath.java ---
@@ -75,7 +75,12 @@ public CarbonTablePath(CarbonTableIdentifier carbonTableIdentifier, String table
* @param carbonFilePath
*/
public static String getFolderContainingFile(String carbonFilePath) {
- return carbonFilePath.substring(0, carbonFilePath.lastIndexOf('/'));
+ int lastIndex = carbonFilePath.lastIndexOf('/');
+ // below code for handling windows environment
+ if (-1 == lastIndex) {
--- End diff --
ok, it's fine
---
[GitHub] carbondata pull request #2004: [CARBONDATA-2208]Pre aggregate datamap creati...
Posted by xubo245 <gi...@git.apache.org>.
Github user xubo245 commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2004#discussion_r170919785
--- Diff: integration/spark2/src/main/scala/org/apache/spark/sql/execution/command/preaaggregate/PreAggregateUtil.scala ---
@@ -346,11 +346,13 @@ object PreAggregateUtil {
carbonTable)
}
// if parent column relation is of size more than one that means aggregate table
- // column is derived from multiple column of main table
- // or if expression is not a instance of attribute reference
+ // column is derived from multiple column of main table or if size is zero then it means
+ // column is present in select statement is some constants for example count(*)
+ // and if expression is not a instance of attribute reference
// then use column name which is passed
val columnName =
- if (parentColumnsName.size > 1 && !expression.isInstanceOf[AttributeReference]) {
+ if ((parentColumnsName.size > 1 || parentColumnsName.isEmpty) &&
+ !expression.isInstanceOf[AttributeReference]) {
newColumnName
--- End diff --
ok, it understand. It's fine, thanks.
---
[GitHub] carbondata pull request #2004: [CARBONDATA-2208]Pre aggregate datamap creati...
Posted by xubo245 <gi...@git.apache.org>.
Github user xubo245 commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2004#discussion_r170902884
--- Diff: core/src/main/java/org/apache/carbondata/core/util/path/CarbonTablePath.java ---
@@ -75,7 +75,12 @@ public CarbonTablePath(CarbonTableIdentifier carbonTableIdentifier, String table
* @param carbonFilePath
*/
public static String getFolderContainingFile(String carbonFilePath) {
- return carbonFilePath.substring(0, carbonFilePath.lastIndexOf('/'));
+ int lastIndex = carbonFilePath.lastIndexOf('/');
+ // below code for handling windows environment
+ if (-1 == lastIndex) {
--- End diff --
Whether it need support this scenario: E:/xubo\idea ?
---
[GitHub] carbondata pull request #2004: [CARBONDATA-2208]Pre aggregate datamap creati...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/carbondata/pull/2004
---
[GitHub] carbondata pull request #2004: [CARBONDATA-2208]Pre aggregate datamap creati...
Posted by xubo245 <gi...@git.apache.org>.
Github user xubo245 commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2004#discussion_r170916327
--- Diff: core/src/main/java/org/apache/carbondata/core/util/path/CarbonTablePath.java ---
@@ -75,7 +75,12 @@ public CarbonTablePath(CarbonTableIdentifier carbonTableIdentifier, String table
* @param carbonFilePath
*/
public static String getFolderContainingFile(String carbonFilePath) {
- return carbonFilePath.substring(0, carbonFilePath.lastIndexOf('/'));
+ int lastIndex = carbonFilePath.lastIndexOf('/');
+ // below code for handling windows environment
+ if (-1 == lastIndex) {
--- End diff --
in windows, you code will return E: if you input E:/xubo\idea , not return E:/xubo
---
[GitHub] carbondata issue #2004: [CARBONDATA-2208]Pre aggregate datamap creation is f...
Posted by chenliang613 <gi...@git.apache.org>.
Github user chenliang613 commented on the issue:
https://github.com/apache/carbondata/pull/2004
LGTM, verified it.
---