-
Notifications
You must be signed in to change notification settings - Fork 29k
[SPARK-11856][SQL] add type cast if the real type is different but compatible with encoder schema #9840
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[SPARK-11856][SQL] add type cast if the real type is different but compatible with encoder schema #9840
Changes from all commits
e9dbd7b
19dbed2
8d6a6ff
e5d963b
7c56223
211a107
6c9dc1e
399d812
2f7370c
57b0d7e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -28,6 +28,7 @@ import org.apache.spark.sql.catalyst.analysis.{SimpleAnalyzer, UnresolvedExtract | |
| import org.apache.spark.sql.catalyst.plans.logical.{LocalRelation, Project} | ||
| import org.apache.spark.sql.catalyst.expressions._ | ||
| import org.apache.spark.sql.catalyst.expressions.codegen.{GenerateSafeProjection, GenerateUnsafeProjection} | ||
| import org.apache.spark.sql.catalyst.optimizer.SimplifyCasts | ||
| import org.apache.spark.sql.catalyst.InternalRow | ||
| import org.apache.spark.sql.catalyst.ScalaReflection | ||
| import org.apache.spark.sql.types.{StructField, ObjectType, StructType} | ||
|
|
@@ -235,12 +236,13 @@ case class ExpressionEncoder[T]( | |
|
|
||
| val plan = Project(Alias(unbound, "")() :: Nil, LocalRelation(schema)) | ||
| val analyzedPlan = SimpleAnalyzer.execute(plan) | ||
| val optimizedPlan = SimplifyCasts(analyzedPlan) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wonder if we should just use the full optimizer here. I guess for now it won't do anything, but since it should never change the answer and we might improve it later that might make more sense.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm also thinking about if we should introduce
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Probably not a On one hand, Scala/Java are always case sensitive so it seems reasonable to preserve that. On the other hand if you loading from something like hive it would be annoying to have to fix all the columns by hand. @rxin, thoughts?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe encoders should be case sensitive all the time to begin with? It is programming language after all, which is case sensitive. If users complain, we can consider adding them in the future?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. SGTM |
||
|
|
||
| // In order to construct instances of inner classes (for example those declared in a REPL cell), | ||
| // we need an instance of the outer scope. This rule substitues those outer objects into | ||
| // expressions that are missing them by looking up the name in the SQLContexts `outerScopes` | ||
| // registry. | ||
| copy(fromRowExpression = analyzedPlan.expressions.head.children.head transform { | ||
| copy(fromRowExpression = optimizedPlan.expressions.head.children.head transform { | ||
| case n: NewInstance if n.outerPointer.isEmpty && n.cls.isMemberClass => | ||
| val outer = outerScopes.get(n.cls.getDeclaringClass.getName) | ||
| if (outer == null) { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here I just use
prettyStringto show the field path likea,a.b, should we also show the type path like we did before?