-
Notifications
You must be signed in to change notification settings - Fork 28k
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-42307][SQL][BUILD][DOCS] Adding in a better name for _LEGACY_ERROR_TEMP_2232
#44337
Conversation
4ef59d2
to
3ff7198
Compare
_LEGACY_ERROR_TEMP_2232
_LEGACY_ERROR_TEMP_2232
@hannahkamundson Could you add a test for the error class, please. |
sql/api/src/test/scala/org/apache/spark/sql/errors/DataTypeErrorsSuite.scala
Outdated
Show resolved
Hide resolved
_LEGACY_ERROR_TEMP_2232
_LEGACY_ERROR_TEMP_2232
sql/api/src/test/scala/org/apache/spark/sql/catalyst/expressions/RowsSuite.scala
Outdated
Show resolved
Hide resolved
@MaxGekk Let me know what you think! |
@MaxGekk Any thoughts? |
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.
@hannahkamundson Could you rebase on the recent master.
|
||
<dependency> | ||
<groupId>org.apache.spark</groupId> | ||
<artifactId>spark-core_${scala.binary.version}</artifactId> |
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.
Please, avoid the dependency.
import org.apache.spark.sql.catalyst.expressions.GenericRow | ||
|
||
class RowSuite extends SparkFunSuite { | ||
test("Row handles null value") { |
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.
Can you move it to some existing test suite?
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.
sql/api
doesn't have any other tests. This was adding the first one.
Do you have a different package you want me to move it to?
Additionally, I think the dependency above is required in test
because SparkFunSuite
lives in core
. I am not putting a dependency for the main code on core
, just the test code. I'm fine changing that, but would need more guidance as to where I should put the test
e0415dc
to
a5e2cd9
Compare
@MaxGekk Rebased on |
We're closing this PR because it hasn't been updated in a while. This isn't a judgement on the merit of the PR in any way. It's just a way of keeping the PR queue manageable. |
What changes were proposed in this pull request?
This adds a better name for the error type
_LEGACY_ERROR_TEMP_2232
and updates the messages for it.Why are the changes needed?
All that was changed was the
_LEGACY_ERROR_TEMP_2232
toNULL_ROW_VALUE
sqlState
for_LEGACY_ERROR_TEMP_2232
_LEGACY_ERROR_TEMP_2232
Does this PR introduce any user-facing change?
Yes.
sqlState
for_LEGACY_ERROR_TEMP_2232
to2200E
whereas it was previously empty._LEGACY_ERROR_TEMP_2232
fromValue at index <index> is null.
toRow value at the position <index> is NULL.
How was this patch tested?
Unit tests
Was this patch authored or co-authored using generative AI tooling?
No