You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@orc.apache.org by GitBox <gi...@apache.org> on 2021/12/20 02:40:43 UTC
[GitHub] [orc] yikf opened a new pull request #973: [ORC-1062]Rename orc.schema.evolution.case.sensitive to orc.schema.evolution.case.aware
yikf opened a new pull request #973:
URL: https://github.com/apache/orc/pull/973
### What changes were proposed in this pull request?
Currently, OrcConf have a enum configuration to determine if the comparision of field names in schema evolution is case sensitive, However, the code block using this configuration thinks that this configuration should be orc.schema.evolution.case.aware
### Why are the changes needed?
code simplification
### How was this patch tested?
Pass GitHub Action and jenkins
--
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: dev-unsubscribe@orc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] yikf commented on pull request #973: [ORC-1062]Rename orc.schema.evolution.case.sensitive to orc.schema.evolution.case.aware
Posted by GitBox <gi...@apache.org>.
yikf commented on pull request #973:
URL: https://github.com/apache/orc/pull/973#issuecomment-997552108
> modify
Done
--
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: dev-unsubscribe@orc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] yikf edited a comment on pull request #973: [ORC-1062]Rename orc.schema.evolution.case.sensitive to orc.schema.evolution.case.aware
Posted by GitBox <gi...@apache.org>.
yikf edited a comment on pull request #973:
URL: https://github.com/apache/orc/pull/973#issuecomment-997547955
@dongjoon-hyun Also, the configuration default value is true, There are some compatibility problems
We have a case, In spark 2.4, the table can be queried normally even if the schema of the table and file is different in case
But in spark 3.1, we need to explicitly configure orc.schema.evolution.case.sensitive as false to query normally.(https://github.com/apache/orc/blob/aee81324507de1c5eb8e86bd7aa93a0f6b02bfd9/java/core/src/java/org/apache/orc/impl/SchemaEvolution.java#L514) this cause `fileIncluded ` not to include this column and cant read data
Whether we should set default value as false to maintain compatibility.
--
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: dev-unsubscribe@orc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] yikf closed pull request #973: [ORC-1062]Rename orc.schema.evolution.case.sensitive to orc.schema.evolution.case.aware
Posted by GitBox <gi...@apache.org>.
yikf closed pull request #973:
URL: https://github.com/apache/orc/pull/973
--
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: dev-unsubscribe@orc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] yikf commented on pull request #973: [ORC-1062]Rename orc.schema.evolution.case.sensitive to orc.schema.evolution.case.aware
Posted by GitBox <gi...@apache.org>.
yikf commented on pull request #973:
URL: https://github.com/apache/orc/pull/973#issuecomment-997552295
> @yikf, Thank you for making this pr, should we also modify this test
>
> https://github.com/apache/orc/blob/aee81324507de1c5eb8e86bd7aa93a0f6b02bfd9/java/core/src/test/org/apache/orc/impl/TestRecordReaderImpl.java#L119-L126
Done
--
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: dev-unsubscribe@orc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] yikf commented on a change in pull request #973: [ORC-1062]Rename orc.schema.evolution.case.sensitive to orc.schema.evolution.case.aware
Posted by GitBox <gi...@apache.org>.
yikf commented on a change in pull request #973:
URL: https://github.com/apache/orc/pull/973#discussion_r772055232
##########
File path: java/core/src/java/org/apache/orc/OrcConf.java
##########
@@ -164,8 +164,8 @@
"testing. Setting this too low may negatively affect performance."),
OVERWRITE_OUTPUT_FILE("orc.overwrite.output.file", "orc.overwrite.output.file", false,
"A boolean flag to enable overwriting of the output file if it already exists.\n"),
- IS_SCHEMA_EVOLUTION_CASE_SENSITIVE("orc.schema.evolution.case.sensitive",
- "orc.schema.evolution.case.sensitive", true,
+ IS_SCHEMA_EVOLUTION_CASE_AWARE("orc.schema.evolution.case.aware",
+ "orc.schema.evolution.case.aware", true,
Review comment:
I'm not sure if this change will affect `hiveConf`
--
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: dev-unsubscribe@orc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] yikf edited a comment on pull request #973: [ORC-1062]Rename orc.schema.evolution.case.sensitive to orc.schema.evolution.case.aware
Posted by GitBox <gi...@apache.org>.
yikf edited a comment on pull request #973:
URL: https://github.com/apache/orc/pull/973#issuecomment-997593138
> Sorry, but I'm -1 for this breaking change, @yikf .
Shall we modify the semantics of the `orc.schema.evolution.case.sensitive`？The default value is true, This indicates that it is not sensitive, But in fact, it is regarded as aware when used
--
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: dev-unsubscribe@orc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] yikf commented on pull request #973: [ORC-1062]Rename orc.schema.evolution.case.sensitive to orc.schema.evolution.case.aware
Posted by GitBox <gi...@apache.org>.
yikf commented on pull request #973:
URL: https://github.com/apache/orc/pull/973#issuecomment-997547955
@dongjoon-hyun Also, the configuration default value is true, There are some compatibility problems
We have a case, In spark 2.4, the table can be queried normally even if the schema of the table and file is different in case
But in spark 3.1, we need to explicitly configure orc.schema.evolution.case.sensitive as false to query normally.(https://github.com/apache/orc/blob/aee81324507de1c5eb8e86bd7aa93a0f6b02bfd9/java/core/src/java/org/apache/orc/impl/SchemaEvolution.java#L514) this cause `fileIncluded ` not to include this column and cant read data
Wether we should set default value as false to maintain compatibility.
--
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: dev-unsubscribe@orc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] guiyanakuang commented on pull request #973: [ORC-1062]Rename orc.schema.evolution.case.sensitive to orc.schema.evolution.case.aware
Posted by GitBox <gi...@apache.org>.
guiyanakuang commented on pull request #973:
URL: https://github.com/apache/orc/pull/973#issuecomment-997550879
@yikf, Thank you for making this pr, should we also modify this test
https://github.com/apache/orc/blob/aee81324507de1c5eb8e86bd7aa93a0f6b02bfd9/java/core/src/test/org/apache/orc/impl/TestRecordReaderImpl.java#L119-L126
--
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: dev-unsubscribe@orc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] yikf commented on pull request #973: [ORC-1062]Rename orc.schema.evolution.case.sensitive to orc.schema.evolution.case.aware
Posted by GitBox <gi...@apache.org>.
yikf commented on pull request #973:
URL: https://github.com/apache/orc/pull/973#issuecomment-997544540
cc @dongjoon-hyun Thanks for your review.
--
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: dev-unsubscribe@orc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] yikf removed a comment on pull request #973: [ORC-1062]Rename orc.schema.evolution.case.sensitive to orc.schema.evolution.case.aware
Posted by GitBox <gi...@apache.org>.
yikf removed a comment on pull request #973:
URL: https://github.com/apache/orc/pull/973#issuecomment-997552108
> modify
Done
--
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: dev-unsubscribe@orc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] yikf commented on a change in pull request #973: [ORC-1062]Rename orc.schema.evolution.case.sensitive to orc.schema.evolution.case.aware
Posted by GitBox <gi...@apache.org>.
yikf commented on a change in pull request #973:
URL: https://github.com/apache/orc/pull/973#discussion_r772061863
##########
File path: java/core/src/java/org/apache/orc/OrcConf.java
##########
@@ -164,8 +164,8 @@
"testing. Setting this too low may negatively affect performance."),
OVERWRITE_OUTPUT_FILE("orc.overwrite.output.file", "orc.overwrite.output.file", false,
"A boolean flag to enable overwriting of the output file if it already exists.\n"),
- IS_SCHEMA_EVOLUTION_CASE_SENSITIVE("orc.schema.evolution.case.sensitive",
- "orc.schema.evolution.case.sensitive", true,
+ IS_SCHEMA_EVOLUTION_CASE_AWARE("orc.schema.evolution.case.aware",
+ "orc.schema.evolution.case.aware", true,
Review comment:
Yes, But there are many changes in this `isSchemaEvolutionCaseAware` to `isSchemaEvolutionCaseSensitive` and the default value also needs to be modified to be consistent with currently one.
If that doesn't affect `hiveConf`, it is better that rename configuration name
--
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: dev-unsubscribe@orc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] yikf commented on pull request #973: [ORC-1062]Rename orc.schema.evolution.case.sensitive to orc.schema.evolution.case.aware
Posted by GitBox <gi...@apache.org>.
yikf commented on pull request #973:
URL: https://github.com/apache/orc/pull/973#issuecomment-997593138
> Sorry, but I'm -1 for this breaking change, @yikf .
Shall we modify the semantics of the `orc.schema.evolution.case.sensitive`？The default value is true, This indicates that it is not sensitive, But in fact, it is regarded as sensitive when used
--
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: dev-unsubscribe@orc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] yikf commented on a change in pull request #973: [ORC-1062]Rename orc.schema.evolution.case.sensitive to orc.schema.evolution.case.aware
Posted by GitBox <gi...@apache.org>.
yikf commented on a change in pull request #973:
URL: https://github.com/apache/orc/pull/973#discussion_r772061863
##########
File path: java/core/src/java/org/apache/orc/OrcConf.java
##########
@@ -164,8 +164,8 @@
"testing. Setting this too low may negatively affect performance."),
OVERWRITE_OUTPUT_FILE("orc.overwrite.output.file", "orc.overwrite.output.file", false,
"A boolean flag to enable overwriting of the output file if it already exists.\n"),
- IS_SCHEMA_EVOLUTION_CASE_SENSITIVE("orc.schema.evolution.case.sensitive",
- "orc.schema.evolution.case.sensitive", true,
+ IS_SCHEMA_EVOLUTION_CASE_AWARE("orc.schema.evolution.case.aware",
+ "orc.schema.evolution.case.aware", true,
Review comment:
Yes, But there are many changes in this `isSchemaEvolutionCaseAware` to `isSchemaEvolutionCaseSensitive` and the default value also needs to be modified to be consistent with currently one.
--
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: dev-unsubscribe@orc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] guiyanakuang commented on pull request #973: [ORC-1062]Rename orc.schema.evolution.case.sensitive to orc.schema.evolution.case.aware
Posted by GitBox <gi...@apache.org>.
guiyanakuang commented on pull request #973:
URL: https://github.com/apache/orc/pull/973#issuecomment-997602180
> The default value is true, This indicates that it is not sensitive
@yikf, It looks like you have some misunderstanding of the source code, the default value is true which means it is sensitive.
--
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: dev-unsubscribe@orc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] guiyanakuang commented on a change in pull request #973: [ORC-1062]Rename orc.schema.evolution.case.sensitive to orc.schema.evolution.case.aware
Posted by GitBox <gi...@apache.org>.
guiyanakuang commented on a change in pull request #973:
URL: https://github.com/apache/orc/pull/973#discussion_r772057996
##########
File path: java/core/src/java/org/apache/orc/OrcConf.java
##########
@@ -164,8 +164,8 @@
"testing. Setting this too low may negatively affect performance."),
OVERWRITE_OUTPUT_FILE("orc.overwrite.output.file", "orc.overwrite.output.file", false,
"A boolean flag to enable overwriting of the output file if it already exists.\n"),
- IS_SCHEMA_EVOLUTION_CASE_SENSITIVE("orc.schema.evolution.case.sensitive",
- "orc.schema.evolution.case.sensitive", true,
+ IS_SCHEMA_EVOLUTION_CASE_AWARE("orc.schema.evolution.case.aware",
+ "orc.schema.evolution.case.aware", true,
Review comment:
So would it be better to change `isSchemaEvolutionCaseAware` to `isSchemaEvolutionCaseSensitive`?
--
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: dev-unsubscribe@orc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] yikf commented on a change in pull request #973: [ORC-1062]Rename orc.schema.evolution.case.sensitive to orc.schema.evolution.case.aware
Posted by GitBox <gi...@apache.org>.
yikf commented on a change in pull request #973:
URL: https://github.com/apache/orc/pull/973#discussion_r772062531
##########
File path: java/core/src/java/org/apache/orc/OrcConf.java
##########
@@ -164,8 +164,8 @@
"testing. Setting this too low may negatively affect performance."),
OVERWRITE_OUTPUT_FILE("orc.overwrite.output.file", "orc.overwrite.output.file", false,
"A boolean flag to enable overwriting of the output file if it already exists.\n"),
- IS_SCHEMA_EVOLUTION_CASE_SENSITIVE("orc.schema.evolution.case.sensitive",
- "orc.schema.evolution.case.sensitive", true,
+ IS_SCHEMA_EVOLUTION_CASE_AWARE("orc.schema.evolution.case.aware",
+ "orc.schema.evolution.case.aware", true,
Review comment:
BTW, Do we think the default value should be insensitive or aware, At present, it is aware
--
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: dev-unsubscribe@orc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org