[GitHub] carbondata pull request #2141: [CARBONDATA-2313] Fixed SDK writer issues and...
Github user asfgit closed the pull request at: https://github.com/apache/carbondata/pull/2141 ---
[GitHub] carbondata pull request #2141: [CARBONDATA-2313] Fixed SDK writer issues and...
Github user ajantha-bhat commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2141#discussion_r181650233
--- Diff:
core/src/main/java/org/apache/carbondata/core/scan/executor/util/RestructureUtil.java
---
@@ -354,7 +369,13 @@ public static Object
getMeasureDefaultValueByType(ColumnSchema columnSchema,
// then setting measure exists is true
// otherwise adding a default value of a measure
for (CarbonMeasure carbonMeasure : currentBlockMeasures) {
-if
(carbonMeasure.getColumnId().equals(queryMeasure.getMeasure().getColumnId())) {
+// If it is unmanaged table just check the column names, no need
to validate column id as
+// multiple sdk's output placed in a single folder doesn't have
same column ID but can
+// have same column name
+if
(carbonMeasure.getColumnId().equals(queryMeasure.getMeasure().getColumnId()) ||
+((queryModel != null) &&
(queryModel.getTable().getTableInfo().isUnManagedTable()) &&
--- End diff --
removed null check. cannot call from a method as one is for measure type
and other is a dimension type.
---
[GitHub] carbondata pull request #2141: [CARBONDATA-2313] Fixed SDK writer issues and...
Github user ajantha-bhat commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2141#discussion_r181648541
--- Diff:
core/src/main/java/org/apache/carbondata/core/scan/executor/util/RestructureUtil.java
---
@@ -337,11 +351,12 @@ public static Object
getMeasureDefaultValueByType(ColumnSchema columnSchema,
* @param blockExecutionInfo
* @param queryMeasuresmeasures present in query
* @param currentBlockMeasures current block measures
+ * @param queryModel carbonQueryModel
* @return measures present in the block
*/
public static List
createMeasureInfoAndGetCurrentBlockQueryMeasures(
BlockExecutionInfo blockExecutionInfo, List
queryMeasures,
- List currentBlockMeasures) {
+ List currentBlockMeasures, QueryModel queryModel) {
--- End diff --
OK
---
[GitHub] carbondata pull request #2141: [CARBONDATA-2313] Fixed SDK writer issues and...
Github user ajantha-bhat commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2141#discussion_r181648562
--- Diff:
core/src/main/java/org/apache/carbondata/core/scan/executor/util/RestructureUtil.java
---
@@ -354,7 +369,13 @@ public static Object
getMeasureDefaultValueByType(ColumnSchema columnSchema,
// then setting measure exists is true
// otherwise adding a default value of a measure
for (CarbonMeasure carbonMeasure : currentBlockMeasures) {
-if
(carbonMeasure.getColumnId().equals(queryMeasure.getMeasure().getColumnId())) {
+// If it is unmanaged table just check the column names, no need
to validate column id as
+// multiple sdk's output placed in a single folder doesn't have
same column ID but can
+// have same column name
+if
(carbonMeasure.getColumnId().equals(queryMeasure.getMeasure().getColumnId()) ||
+((queryModel != null) &&
(queryModel.getTable().getTableInfo().isUnManagedTable()) &&
--- End diff --
ok
---
[GitHub] carbondata pull request #2141: [CARBONDATA-2313] Fixed SDK writer issues and...
Github user ajantha-bhat commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2141#discussion_r181647929
--- Diff:
integration/spark-common-test/src/test/scala/org/apache/carbondata/spark/testsuite/createTable/TestUnmanagedCarbonTable.scala
---
@@ -266,16 +270,23 @@ class TestUnmanagedCarbonTable extends QueryTest with
BeforeAndAfterAll {
.contains("Unsupported operation on unmanaged table"))
//12. Streaming table creation
-// External table don't accept table properties
+// No need as External table don't accept table properties
+
+//13. Alter table rename command
+exception = intercept[MalformedCarbonCommandException] {
+ sql("ALTER TABLE sdkOutputTable RENAME to newTable")
+}
+assert(exception.getMessage()
+ .contains("Unsupported operation on unmanaged table"))
sql("DROP TABLE sdkOutputTable")
-// drop table should not delete the files
+ //drop table should not delete the files
--- End diff --
done
---
[GitHub] carbondata pull request #2141: [CARBONDATA-2313] Fixed SDK writer issues and...
Github user ajantha-bhat commented on a diff in the pull request: https://github.com/apache/carbondata/pull/2141#discussion_r181646642 --- Diff: integration/spark2/src/main/scala/org/apache/spark/sql/execution/command/schema/CarbonAlterTableRenameCommand.scala --- @@ -73,22 +73,27 @@ private[sql] case class CarbonAlterTableRenameCommand( s"Table $oldDatabaseName.$oldTableName does not exist") throwMetadataException(oldDatabaseName, oldTableName, "Table does not exist") } + +var carbonTable: CarbonTable = null +carbonTable = metastore.lookupRelation(Some(oldDatabaseName), oldTableName)(sparkSession) --- End diff -- OK. modified ---
[GitHub] carbondata pull request #2141: [CARBONDATA-2313] Fixed SDK writer issues and...
Github user ajantha-bhat commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2141#discussion_r181646540
--- Diff:
integration/spark2/src/main/scala/org/apache/spark/sql/hive/CarbonFileMetastore.scala
---
@@ -565,7 +563,9 @@ class CarbonFileMetastore extends CarbonMetaStore {
tableModifiedTimeStore.get(CarbonCommonConstants.DATABASE_DEFAULT_NAME))) {
metadata.carbonTables = metadata.carbonTables.filterNot(
table =>
table.getTableName.equalsIgnoreCase(tableIdentifier.table) &&
-
table.getDatabaseName.equalsIgnoreCase(tableIdentifier.database.getOrElse("default")))
+ table.getDatabaseName.equalsIgnoreCase(
--- End diff --
Ran all the test case. It is not coming now. Removed this changes
---
[GitHub] carbondata pull request #2141: [CARBONDATA-2313] Fixed SDK writer issues and...
Github user gvramana commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2141#discussion_r181637617
--- Diff:
integration/spark2/src/main/scala/org/apache/spark/sql/hive/CarbonFileMetastore.scala
---
@@ -565,7 +563,9 @@ class CarbonFileMetastore extends CarbonMetaStore {
tableModifiedTimeStore.get(CarbonCommonConstants.DATABASE_DEFAULT_NAME))) {
metadata.carbonTables = metadata.carbonTables.filterNot(
table =>
table.getTableName.equalsIgnoreCase(tableIdentifier.table) &&
-
table.getDatabaseName.equalsIgnoreCase(tableIdentifier.database.getOrElse("default")))
+ table.getDatabaseName.equalsIgnoreCase(
--- End diff --
Why it coming here for unmanged table
---
[GitHub] carbondata pull request #2141: [CARBONDATA-2313] Fixed SDK writer issues and...
Github user gvramana commented on a diff in the pull request: https://github.com/apache/carbondata/pull/2141#discussion_r181634921 --- Diff: integration/spark2/src/main/scala/org/apache/spark/sql/execution/command/schema/CarbonAlterTableRenameCommand.scala --- @@ -73,22 +73,27 @@ private[sql] case class CarbonAlterTableRenameCommand( s"Table $oldDatabaseName.$oldTableName does not exist") throwMetadataException(oldDatabaseName, oldTableName, "Table does not exist") } + +var carbonTable: CarbonTable = null +carbonTable = metastore.lookupRelation(Some(oldDatabaseName), oldTableName)(sparkSession) --- End diff -- This cannot be moved outside lock ---
[GitHub] carbondata pull request #2141: [CARBONDATA-2313] Fixed SDK writer issues and...
Github user gvramana commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2141#discussion_r181634024
--- Diff:
integration/spark-common-test/src/test/scala/org/apache/carbondata/spark/testsuite/createTable/TestUnmanagedCarbonTable.scala
---
@@ -266,16 +270,23 @@ class TestUnmanagedCarbonTable extends QueryTest with
BeforeAndAfterAll {
.contains("Unsupported operation on unmanaged table"))
//12. Streaming table creation
-// External table don't accept table properties
+// No need as External table don't accept table properties
+
+//13. Alter table rename command
+exception = intercept[MalformedCarbonCommandException] {
+ sql("ALTER TABLE sdkOutputTable RENAME to newTable")
+}
+assert(exception.getMessage()
+ .contains("Unsupported operation on unmanaged table"))
sql("DROP TABLE sdkOutputTable")
-// drop table should not delete the files
+ //drop table should not delete the files
--- End diff --
correct indent
---
[GitHub] carbondata pull request #2141: [CARBONDATA-2313] Fixed SDK writer issues and...
Github user gvramana commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2141#discussion_r181632750
--- Diff:
core/src/main/java/org/apache/carbondata/core/scan/executor/util/RestructureUtil.java
---
@@ -354,7 +369,13 @@ public static Object
getMeasureDefaultValueByType(ColumnSchema columnSchema,
// then setting measure exists is true
// otherwise adding a default value of a measure
for (CarbonMeasure carbonMeasure : currentBlockMeasures) {
-if
(carbonMeasure.getColumnId().equals(queryMeasure.getMeasure().getColumnId())) {
+// If it is unmanaged table just check the column names, no need
to validate column id as
+// multiple sdk's output placed in a single folder doesn't have
same column ID but can
+// have same column name
+if
(carbonMeasure.getColumnId().equals(queryMeasure.getMeasure().getColumnId()) ||
+((queryModel != null) &&
(queryModel.getTable().getTableInfo().isUnManagedTable()) &&
--- End diff --
remove null check and add method isColumnMatches
---
[GitHub] carbondata pull request #2141: [CARBONDATA-2313] Fixed SDK writer issues and...
Github user gvramana commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2141#discussion_r181632323
--- Diff:
core/src/main/java/org/apache/carbondata/core/scan/executor/util/RestructureUtil.java
---
@@ -337,11 +351,12 @@ public static Object
getMeasureDefaultValueByType(ColumnSchema columnSchema,
* @param blockExecutionInfo
* @param queryMeasuresmeasures present in query
* @param currentBlockMeasures current block measures
+ * @param queryModel carbonQueryModel
* @return measures present in the block
*/
public static List
createMeasureInfoAndGetCurrentBlockQueryMeasures(
BlockExecutionInfo blockExecutionInfo, List
queryMeasures,
- List currentBlockMeasures) {
+ List currentBlockMeasures, QueryModel queryModel) {
--- End diff --
Pass flag, dont pass all the querymodel, Restructuring doesnot have any
relation with Querymodel
---
