Skip to content

Conversation

@lvshaokang
Copy link
Contributor

What changes were proposed in this pull request?

In the PR, I propose to use error classes in the case of type check failure in Bloom Filter Agg expressions.

Why are the changes needed?

Migration onto error classes unifies Spark SQL error messages.

Does this PR introduce any user-facing change?

Yes. The PR changes user-facing error messages.

How was this patch tested?

build/sbt "sql/testOnly *SQLQueryTestSuite"
build/sbt "test:testOnly org.apache.spark.SparkThrowableSuite"
build/sbt "test:testOnly *BloomFilterAggregateQuerySuite"

@lvshaokang
Copy link
Contributor Author

cc @MaxGekk

@MaxGekk
Copy link
Member

MaxGekk commented Oct 20, 2022

@lvshaokang Isn't the failure related to your changes?

Different files: ['base_pb2.pyi', 'commands_pb2.pyi', 'expressions_pb2.pyi', 'relations_pb2.pyi', 'types_pb2.pyi']
Generated files for pyspark-connect are out of sync! Please run ./connector/connect/dev/generate_protos.sh
Error: Process completed with exit code 255.

@lvshaokang
Copy link
Contributor Author

@MaxGekk No, It seems to be unrelated, I don't know why it would lead to this, and I tried retrying CI, but it still reports an error.

@MaxGekk
Copy link
Member

MaxGekk commented Oct 20, 2022

@HyukjinKwon @zhengruifeng @grundprinzip Should we worry about the failures #38315 (comment) or can just ignore them?

@grundprinzip
Copy link
Contributor

@HyukjinKwon @zhengruifeng @grundprinzip Should we worry about the failures #38315 (comment) or can just ignore them?

In theory, of you merge master into your branch this should go away as current master should be in sync.

@lvshaokang
Copy link
Contributor Author

In theory, of you merge master into your branch this should go away as current master should be in sync.

I have merged the master branch into my branch and push it, wating the CI complete.

@AmplabJenkins
Copy link

Can one of the admins verify this patch?

@lvshaokang
Copy link
Contributor Author

@MaxGekk CI run successful. Please take a look, thanks.

@MaxGekk
Copy link
Member

MaxGekk commented Oct 21, 2022

+1, LGTM. Merging to master.
Thank you, @lvshaokang.

@MaxGekk MaxGekk closed this in 11cce7e Oct 21, 2022
@zhengruifeng
Copy link
Contributor

@HyukjinKwon @zhengruifeng @grundprinzip Should we worry about the failures #38315 (comment) or can just ignore them?

that was due to mypy-protobuf upgrade, now the version was pined in 0643d02

SandishKumarHN pushed a commit to SandishKumarHN/spark that referenced this pull request Dec 12, 2022
…onto error classes

### What changes were proposed in this pull request?

In the PR, I propose to use error classes in the case of type check failure in Bloom Filter Agg expressions.

### Why are the changes needed?

Migration onto error classes unifies Spark SQL error messages.

### Does this PR introduce _any_ user-facing change?

Yes. The PR changes user-facing error messages.

### How was this patch tested?

```
build/sbt "sql/testOnly *SQLQueryTestSuite"
build/sbt "test:testOnly org.apache.spark.SparkThrowableSuite"
build/sbt "test:testOnly *BloomFilterAggregateQuerySuite"
```

Closes apache#38315 from lvshaokang/SPARK-40768.

Authored-by: lvshaokang <[email protected]>
Signed-off-by: Max Gekk <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants