[CDAP-15361] Add Schema handling in Wrangler #645
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
tivv
reviewed
Jun 22, 2023
wrangler-api/src/main/java/io/cdap/wrangler/api/schema/SchemaFieldGenerator.java
Outdated
Show resolved
Hide resolved
iam-divya
previously requested changes
Jun 23, 2023
wrangler-api/src/main/java/io/cdap/wrangler/api/schema/SchemaFieldGenerator.java
Outdated
Show resolved
Hide resolved
wrangler-core/src/main/java/io/cdap/wrangler/executor/RecipePipelineExecutor.java
Outdated
Show resolved
Hide resolved
wrangler-core/src/main/java/io/cdap/wrangler/executor/RecipePipelineExecutor.java
Outdated
Show resolved
Hide resolved
iam-divya
reviewed
Jun 23, 2023
wrangler-core/src/main/java/io/cdap/directives/column/Rename.java
Outdated
Show resolved
Hide resolved
vanathi-g
changed the title
tivv
reviewed
Jun 23, 2023
tivv
requested changes
Jun 28, 2023
wrangler-api/src/main/java/io/cdap/wrangler/api/schema/SchemaUpdater.java
Outdated
Show resolved
Hide resolved
tivv
reviewed
Jun 28, 2023
wrangler-api/src/main/java/io/cdap/wrangler/api/schema/SchemaUpdater.java
Outdated
Show resolved
Hide resolved
tivv
reviewed
Jul 5, 2023
wrangler-core/src/main/java/io/cdap/wrangler/executor/RecipePipelineExecutor.java
Outdated
Show resolved
Hide resolved
wrangler-core/src/main/java/io/cdap/wrangler/executor/RecipePipelineExecutor.java
Outdated
Show resolved
Hide resolved
wrangler-core/src/main/java/io/cdap/wrangler/executor/RecipePipelineExecutor.java
Outdated
Show resolved
Hide resolved
wrangler-core/src/main/java/io/cdap/wrangler/schema/Constants.java
Outdated
Show resolved
Hide resolved
wrangler-core/src/main/java/io/cdap/wrangler/schema/Constants.java
Outdated
Show resolved
Hide resolved
wrangler-core/src/main/java/io/cdap/wrangler/executor/RecipePipelineExecutor.java
Outdated
Show resolved
Hide resolved
wrangler-core/src/main/java/io/cdap/wrangler/executor/RecipePipelineExecutor.java
Outdated
Show resolved
Hide resolved
wrangler-core/src/main/java/io/cdap/wrangler/executor/RecipePipelineExecutor.java
Outdated
Show resolved
Hide resolved
wrangler-service/src/main/java/io/cdap/wrangler/service/directive/AbstractDirectiveHandler.java
Outdated
Show resolved
Hide resolved
wrangler-service/src/main/java/io/cdap/wrangler/service/directive/AbstractDirectiveHandler.java
Outdated
Show resolved
Hide resolved
saimukkamala
approved these changes
Jul 13, 2023
tivv
reviewed
Jul 14, 2023
wrangler-core/src/main/java/io/cdap/wrangler/executor/RecipePipelineExecutor.java
Outdated
Show resolved
Hide resolved
wrangler-core/src/main/java/io/cdap/wrangler/executor/RecipePipelineExecutor.java
Outdated
Show resolved
Hide resolved
| if (designTime && schema != null) { | ||
| Schema directiveOutputSchema = directive.getOutputSchema(schema); | ||
| schema = directiveOutputSchema != null ? directiveOutputSchema | ||
| : generateOutputSchema(schema, cumulativeRows); |
There was a problem hiding this comment.
We should introduce a feature flag for this with default enabled.. If schema generator fails on some customer schema, there should be a way to disable it
wrangler-core/src/main/java/io/cdap/wrangler/executor/RecipePipelineExecutor.java
Outdated
Show resolved
Hide resolved
wrangler-core/src/main/java/io/cdap/wrangler/executor/RecipePipelineExecutor.java
Outdated
Show resolved
Hide resolved
tivv
reviewed
Jul 14, 2023
tivv
reviewed
Jul 21, 2023
wrangler-core/src/main/java/io/cdap/wrangler/executor/RecipePipelineExecutor.java
Outdated
Show resolved
Hide resolved
wrangler-core/src/main/java/io/cdap/wrangler/utils/OutputSchemaGenerator.java
Outdated
Show resolved
Hide resolved
tivv
reviewed
Jul 24, 2023
wrangler-core/src/main/java/io/cdap/wrangler/utils/DirectiveOutputSchemaGenerator.java
Outdated
Show resolved
Hide resolved
wrangler-core/src/main/java/io/cdap/wrangler/utils/DirectiveOutputSchemaGenerator.java
Outdated
Show resolved
Hide resolved
wrangler-core/src/main/java/io/cdap/wrangler/utils/DirectiveOutputSchemaGenerator.java
Outdated
Show resolved
Hide resolved
wrangler-core/src/main/java/io/cdap/wrangler/utils/DirectiveOutputSchemaGenerator.java
Outdated
Show resolved
Hide resolved
| Map<String, String> types = new LinkedHashMap<>(); | ||
|
|
||
| Schema outputSchema = TRANSIENT_STORE.get(TransientStoreKeys.OUTPUT_SCHEMA) != null ? | ||
| TRANSIENT_STORE.get(TransientStoreKeys.OUTPUT_SCHEMA) : TRANSIENT_STORE.get(TransientStoreKeys.INPUT_SCHEMA); |
There was a problem hiding this comment.
In which cases OUTPUT_SCHEMA will be null? This "else" part seems problematic for me.
There was a problem hiding this comment.
Initially only input schema is set in transient store (when workspace created for ex)
tivv
approved these changes
Jul 24, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Background
Currently, Wrangler infers the CDAP schema of the data where necessary during design-time (UI display, creating plugin/applying directives). This leads to inference errors and information loss. Hence, a mechanism for directives to correctly update schema after transformation is implemented and input schema information is not lost.
Changes
getOutputSchemamethod to Executor interface (Directives can use this to return custom schema after transform)TransientStoreand accessed when necessary.specificationand AbstractDirectiveHandler'sgenerateExecutionResponsemethod now use the output schema instead of inferringgetOutputSchema, the schema is generated as a fallback mechanismNOTE: the schema changes are only applicable in design time (when Wrangler runs as a service, not in the plugin)