[GitHub] incubator-carbondata pull request #528: [CARBONDATA-617]Fix InsertInto test ...
Github user asfgit closed the pull request at: https://github.com/apache/incubator-carbondata/pull/528 --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-carbondata pull request #528: [CARBONDATA-617]Fix InsertInto test ...
Github user QiangCai commented on a diff in the pull request: https://github.com/apache/incubator-carbondata/pull/528#discussion_r96129389 --- Diff: processing/src/main/java/org/apache/carbondata/processing/csvreaderstep/CsvInput.java --- @@ -194,28 +196,25 @@ public boolean processRow(StepMetaInterface smi, StepDataInterface sdi) throws K } class RddScanCallable implements Callable { -List> iterList; - -RddScanCallable() { - this.iterList = new ArrayList >(1000); -} - -public void addJavaRddIterator(CarbonIterator iter) { - this.iterList.add(iter); -} - -@Override -public Void call() throws Exception { - StandardLogService.setThreadName(("PROCESS_DataFrame_PARTITIONS"), - Thread.currentThread().getName()); +@Override public Void call() throws Exception { + StandardLogService + .setThreadName(("PROCESS_DataFrame_PARTITIONS"), Thread.currentThread().getName()); try { String[] values = null; -for (CarbonIterator iter: iterList) { - iter.initialize(); - while (iter.hasNext()) { -values = iter.next(); -synchronized (putRowLock) { - putRow(data.outputRowMeta, values); +boolean hasNext = true; +CarbonIterator iter; +boolean isInitialized = false; +while (hasNext) { + iter = getRddIterator(isInitialized); --- End diff -- ok --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-carbondata pull request #528: [CARBONDATA-617]Fix InsertInto test ...
Github user QiangCai commented on a diff in the pull request: https://github.com/apache/incubator-carbondata/pull/528#discussion_r96129390 --- Diff: integration/spark2/src/main/scala/org/apache/spark/sql/hive/CarbonMetastore.scala --- @@ -782,7 +782,47 @@ case class CarbonRelation( nullable = true)()) } - override val output = dimensionsAttr ++ measureAttr + override val output = { +val columns = tableMeta.carbonTable.getCreateOrderColumn(tableMeta.carbonTable.getFactTableName) + .asScala +columns.filter(!_.isInvisible).map { column => --- End diff -- ok --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-carbondata pull request #528: [CARBONDATA-617]Fix InsertInto test ...
Github user jackylk commented on a diff in the pull request: https://github.com/apache/incubator-carbondata/pull/528#discussion_r96108506 --- Diff: integration/spark2/src/main/scala/org/apache/spark/sql/hive/CarbonMetastore.scala --- @@ -782,7 +782,47 @@ case class CarbonRelation( nullable = true)()) } - override val output = dimensionsAttr ++ measureAttr + override val output = { +val columns = tableMeta.carbonTable.getCreateOrderColumn(tableMeta.carbonTable.getFactTableName) + .asScala +columns.filter(!_.isInvisible).map { column => --- End diff -- can you add comment in code to describe this conversion --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-carbondata pull request #528: [CARBONDATA-617]Fix InsertInto test ...
Github user jackylk commented on a diff in the pull request: https://github.com/apache/incubator-carbondata/pull/528#discussion_r96108503 --- Diff: integration/spark2/src/main/scala/org/apache/spark/sql/hive/CarbonMetastore.scala --- @@ -782,7 +782,47 @@ case class CarbonRelation( nullable = true)()) } - override val output = dimensionsAttr ++ measureAttr + override val output = { +val columns = tableMeta.carbonTable.getCreateOrderColumn(tableMeta.carbonTable.getFactTableName) + .asScala +columns.filter(!_.isInvisible).map { column => + if (column.isDimesion()) { +val output: DataType = column.getDataType.toString.toLowerCase match { + case "array" => + CarbonMetastoreTypes.toDataType(s"array<${getArrayChildren(column.getColName)}>") + case "struct" => + CarbonMetastoreTypes.toDataType(s"struct<${getStructChildren(column.getColName)}>") + case dType => +val dataType = addDecimalScaleAndPrecision(column, dType) +CarbonMetastoreTypes.toDataType(dataType) +} +AttributeReference(column.getColName, output, nullable = true )( + qualifier = Option(tableName + "." + column.getColName)) + } else { +val output = CarbonMetastoreTypes.toDataType { --- End diff -- can you add comment in code to describe this conversion --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-carbondata pull request #528: [CARBONDATA-617]Fix InsertInto test ...
Github user jackylk commented on a diff in the pull request: https://github.com/apache/incubator-carbondata/pull/528#discussion_r96108486 --- Diff: processing/src/main/java/org/apache/carbondata/processing/csvreaderstep/CsvInput.java --- @@ -194,28 +196,25 @@ public boolean processRow(StepMetaInterface smi, StepDataInterface sdi) throws K } class RddScanCallable implements Callable { -List> iterList; - -RddScanCallable() { - this.iterList = new ArrayList >(1000); -} - -public void addJavaRddIterator(CarbonIterator iter) { - this.iterList.add(iter); -} - -@Override -public Void call() throws Exception { - StandardLogService.setThreadName(("PROCESS_DataFrame_PARTITIONS"), - Thread.currentThread().getName()); +@Override public Void call() throws Exception { + StandardLogService + .setThreadName(("PROCESS_DataFrame_PARTITIONS"), Thread.currentThread().getName()); try { String[] values = null; -for (CarbonIterator iter: iterList) { - iter.initialize(); - while (iter.hasNext()) { -values = iter.next(); -synchronized (putRowLock) { - putRow(data.outputRowMeta, values); +boolean hasNext = true; +CarbonIterator iter; +boolean isInitialized = false; +while (hasNext) { + iter = getRddIterator(isInitialized); --- End diff -- can you provide some comment in the code to describe this while loop --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-carbondata pull request #528: [CARBONDATA-617]Fix InsertInto test ...
Github user QiangCai commented on a diff in the pull request: https://github.com/apache/incubator-carbondata/pull/528#discussion_r95973907 --- Diff: integration/spark2/src/main/scala/org/apache/spark/sql/hive/CarbonMetastore.scala --- @@ -782,7 +781,49 @@ case class CarbonRelation( nullable = true)()) } - override val output = dimensionsAttr ++ measureAttr + override val output = { +val columns = tableMeta.carbonTable.getCreateOrderColumn(tableMeta.carbonTable.getFactTableName) + .asScala +columns.filter(!_.isInvisible).map { column => + if (column.isDimesion()) { +val output: DataType = column.getDataType.toString.toLowerCase match { + case "array" => + CarbonMetastoreTypes.toDataType(s"array<${getArrayChildren(column.getColName)}>") + case "struct" => + CarbonMetastoreTypes.toDataType(s"struct<${getStructChildren(column.getColName)}>") + case dType => +val dataType = addDecimalScaleAndPrecision(column, dType) +CarbonMetastoreTypes.toDataType(dataType) +} +AttributeReference(column.getColName, output, + nullable = true +)(qualifier = Option(tableName + "." + column.getColName)) --- End diff -- fixed --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-carbondata pull request #528: [CARBONDATA-617]Fix InsertInto test ...
Github user QiangCai commented on a diff in the pull request: https://github.com/apache/incubator-carbondata/pull/528#discussion_r95973885 --- Diff: integration/spark/src/main/scala/org/apache/spark/sql/optimizer/CarbonOptimizer.scala --- @@ -237,9 +237,15 @@ class ResolveCarbonFunctions(relations: Seq[CarbonDecoderRelation]) val leftCondAttrs = new util.HashSet[AttributeReferenceWrapper] val rightCondAttrs = new util.HashSet[AttributeReferenceWrapper] union.left.output.foreach(attr => --- End diff -- fixed --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-carbondata pull request #528: [CARBONDATA-617]Fix InsertInto test ...
Github user jackylk commented on a diff in the pull request: https://github.com/apache/incubator-carbondata/pull/528#discussion_r95956920 --- Diff: integration/spark2/src/main/scala/org/apache/spark/sql/hive/CarbonMetastore.scala --- @@ -782,7 +781,49 @@ case class CarbonRelation( nullable = true)()) } - override val output = dimensionsAttr ++ measureAttr + override val output = { +val columns = tableMeta.carbonTable.getCreateOrderColumn(tableMeta.carbonTable.getFactTableName) + .asScala +columns.filter(!_.isInvisible).map { column => + if (column.isDimesion()) { +val output: DataType = column.getDataType.toString.toLowerCase match { + case "array" => + CarbonMetastoreTypes.toDataType(s"array<${getArrayChildren(column.getColName)}>") + case "struct" => + CarbonMetastoreTypes.toDataType(s"struct<${getStructChildren(column.getColName)}>") + case dType => +val dataType = addDecimalScaleAndPrecision(column, dType) +CarbonMetastoreTypes.toDataType(dataType) +} +AttributeReference(column.getColName, output, + nullable = true +)(qualifier = Option(tableName + "." + column.getColName)) + } else { +AttributeReference(column.getColName, CarbonMetastoreTypes.toDataType( + column.getDataType.toString +.toLowerCase match { +case "int" => "long" +case "short" => "long" +case "decimal" => "decimal(" + column.getColumnSchema.getPrecision + "," + column + .getColumnSchema.getScale + ")" +case others => others + } +), + nullable = true +)(qualifier = Option(tableName + "." + column.getColName)) + } +} + } + + def addDecimalScaleAndPrecision(dimval: CarbonColumn, dataType: String): String = { +var dType = dataType +if (dimval.getDataType +== org.apache.carbondata.core.carbon.metadata.datatype.DataType.DECIMAL) { --- End diff -- remove the explicit package name --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-carbondata pull request #528: [CARBONDATA-617]Fix InsertInto test ...
Github user jackylk commented on a diff in the pull request: https://github.com/apache/incubator-carbondata/pull/528#discussion_r95956880 --- Diff: integration/spark2/src/main/scala/org/apache/spark/sql/hive/CarbonMetastore.scala --- @@ -782,7 +781,49 @@ case class CarbonRelation( nullable = true)()) } - override val output = dimensionsAttr ++ measureAttr + override val output = { +val columns = tableMeta.carbonTable.getCreateOrderColumn(tableMeta.carbonTable.getFactTableName) + .asScala +columns.filter(!_.isInvisible).map { column => + if (column.isDimesion()) { +val output: DataType = column.getDataType.toString.toLowerCase match { + case "array" => + CarbonMetastoreTypes.toDataType(s"array<${getArrayChildren(column.getColName)}>") + case "struct" => + CarbonMetastoreTypes.toDataType(s"struct<${getStructChildren(column.getColName)}>") + case dType => +val dataType = addDecimalScaleAndPrecision(column, dType) +CarbonMetastoreTypes.toDataType(dataType) +} +AttributeReference(column.getColName, output, + nullable = true +)(qualifier = Option(tableName + "." + column.getColName)) + } else { +AttributeReference(column.getColName, CarbonMetastoreTypes.toDataType( --- End diff -- move `CarbonMetastoreTypes.toDataType` to previous line to make it more readable --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-carbondata pull request #528: [CARBONDATA-617]Fix InsertInto test ...
Github user jackylk commented on a diff in the pull request: https://github.com/apache/incubator-carbondata/pull/528#discussion_r95956802 --- Diff: integration/spark2/src/main/scala/org/apache/spark/sql/hive/CarbonMetastore.scala --- @@ -782,7 +781,49 @@ case class CarbonRelation( nullable = true)()) } - override val output = dimensionsAttr ++ measureAttr + override val output = { +val columns = tableMeta.carbonTable.getCreateOrderColumn(tableMeta.carbonTable.getFactTableName) + .asScala +columns.filter(!_.isInvisible).map { column => + if (column.isDimesion()) { +val output: DataType = column.getDataType.toString.toLowerCase match { + case "array" => + CarbonMetastoreTypes.toDataType(s"array<${getArrayChildren(column.getColName)}>") + case "struct" => + CarbonMetastoreTypes.toDataType(s"struct<${getStructChildren(column.getColName)}>") + case dType => +val dataType = addDecimalScaleAndPrecision(column, dType) +CarbonMetastoreTypes.toDataType(dataType) +} +AttributeReference(column.getColName, output, + nullable = true +)(qualifier = Option(tableName + "." + column.getColName)) --- End diff -- move to previous line --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-carbondata pull request #528: [CARBONDATA-617]Fix InsertInto test ...
Github user jackylk commented on a diff in the pull request: https://github.com/apache/incubator-carbondata/pull/528#discussion_r95956671 --- Diff: integration/spark/src/main/scala/org/apache/spark/sql/optimizer/CarbonOptimizer.scala --- @@ -237,9 +237,15 @@ class ResolveCarbonFunctions(relations: Seq[CarbonDecoderRelation]) val leftCondAttrs = new util.HashSet[AttributeReferenceWrapper] val rightCondAttrs = new util.HashSet[AttributeReferenceWrapper] union.left.output.foreach(attr => --- End diff -- use `{` instead of `(` --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-carbondata pull request #528: [CARBONDATA-617]Fix InsertInto test ...
Github user jackylk commented on a diff in the pull request: https://github.com/apache/incubator-carbondata/pull/528#discussion_r95944775 --- Diff: processing/src/main/java/org/apache/carbondata/processing/csvreaderstep/CsvInput.java --- @@ -193,28 +195,26 @@ public boolean processRow(StepMetaInterface smi, StepDataInterface sdi) throws K } class RddScanCallable implements Callable { -List> iterList; - -RddScanCallable() { - this.iterList = new ArrayList >(1000); -} - -public void addJavaRddIterator(JavaRddIterator iter) { - this.iterList.add(iter); -} - @Override public Void call() throws Exception { StandardLogService.setThreadName(("PROCESS_DataFrame_PARTITIONS"), Thread.currentThread().getName()); try { String[] values = null; -for (JavaRddIterator iter: iterList) { - iter.initialize(); - while (iter.hasNext()) { -values = iter.next(); -synchronized (putRowLock) { - putRow(data.outputRowMeta, values); +boolean hasNext = true; +JavaRddIterator iter; +boolean isInitialized = false; +while(hasNext) { --- End diff -- please add space after while --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---