github-actions[bot] closed pull request #10766: Add `IcebergAnalysisException`
in iceberg-spark module
URL: https://github.com/apache/iceberg/pull/10766
--
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
github-actions[bot] commented on PR #10766:
URL: https://github.com/apache/iceberg/pull/10766#issuecomment-2480862614
This pull request has been closed due to lack of activity. This is not a
judgement on the merit of the PR in any way. It is just a way of keeping the PR
queue manageable. If
github-actions[bot] commented on PR #10766:
URL: https://github.com/apache/iceberg/pull/10766#issuecomment-2466519741
This pull request has been marked as stale due to 30 days of inactivity. It
will be closed in 1 week if no further activity occurs. If you think that’s
incorrect or this pul
huaxingao commented on PR #10766:
URL: https://github.com/apache/iceberg/pull/10766#issuecomment-2253612812
> We haven't written this down (I think) but we generally have similar rules
that we should probably publish.
I will check to see if we need a doc PR for this
--
This is an a
huaxingao commented on PR #10766:
URL: https://github.com/apache/iceberg/pull/10766#issuecomment-2253607788
> Do we have to do this in 3.5 or is it only required for 4.0?
@RussellSpitzer Thanks for reviewing the PR! We don't have to do this in
3.5. I was just trying to separate a few
RussellSpitzer commented on PR #10766:
URL: https://github.com/apache/iceberg/pull/10766#issuecomment-2253554397
My only worry here is that RevAPI is missing this as a public api method
change because it's in Scala but we are changing the named exception being
thrown and thus changing a met
RussellSpitzer commented on PR #10766:
URL: https://github.com/apache/iceberg/pull/10766#issuecomment-2253550137
@huaxingao Do we have to do this in 3.5 or is it only required for 4.0?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to
RussellSpitzer commented on PR #10766:
URL: https://github.com/apache/iceberg/pull/10766#issuecomment-2253547485
> This makes sense, let me take a deeper look tomorrow. I think in a
subsequent pr we can make a pass in the error messages to try to follow:
https://spark.apache.org/error-messa
szehon-ho commented on PR #10766:
URL: https://github.com/apache/iceberg/pull/10766#issuecomment-2251536703
I think im ok with it. I slightly prefer to not have this until 4.0, as it
doesnt seem that necessary , but I think it will not hurt. Per
https://github.com/apache/spark/commit/3e08
szehon-ho commented on code in PR #10766:
URL: https://github.com/apache/iceberg/pull/10766#discussion_r1692245109
##
spark/v3.5/spark/src/main/java/org/apache/spark/sql/catalyst/analysis/IcebergAnalysisException.java:
##
@@ -0,0 +1,37 @@
+/*
+ * Licensed to the Apache Software
szehon-ho commented on code in PR #10766:
URL: https://github.com/apache/iceberg/pull/10766#discussion_r1692091564
##
spark/v3.5/spark/src/main/java/org/apache/spark/sql/catalyst/analysis/IcebergAnalysisException.java:
##
@@ -0,0 +1,37 @@
+/*
+ * Licensed to the Apache Software
huaxingao commented on PR #10766:
URL: https://github.com/apache/iceberg/pull/10766#issuecomment-2249186701
> Curious what's the plan for Spark 4.0 then?
I can't find a formally written down plan for this.
> Just brainstorming out loud the value of IcebergAnalysisException, anot
huaxingao commented on code in PR #10766:
URL: https://github.com/apache/iceberg/pull/10766#discussion_r1690665089
##
spark/v3.5/spark/src/main/java/org/apache/spark/sql/catalyst/analysis/IcebergAnalysisException.java:
##
@@ -0,0 +1,37 @@
+/*
+ * Licensed to the Apache Software
szehon-ho commented on PR #10766:
URL: https://github.com/apache/iceberg/pull/10766#issuecomment-2249088638
> Spark 4.0 requires AnalysisException to have error class
Curious what's the plan for Spark 4.0 then?
Just brainstorming out loud the value of IcebergAnalysisException,
szehon-ho commented on code in PR #10766:
URL: https://github.com/apache/iceberg/pull/10766#discussion_r1690602060
##
spark/v3.5/spark/src/main/java/org/apache/spark/sql/catalyst/analysis/IcebergAnalysisException.java:
##
@@ -0,0 +1,37 @@
+/*
+ * Licensed to the Apache Software
szehon-ho commented on PR #10766:
URL: https://github.com/apache/iceberg/pull/10766#issuecomment-2247135039
This makes sense, let me take a deeper look tomorrow. I think in a
subsequent pr we can make a pass in the error messages to try to follow:
https://spark.apache.org/error-message-gu
huaxingao commented on PR #10766:
URL: https://github.com/apache/iceberg/pull/10766#issuecomment-2247098069
When constructing a new `AnalysisException`, if we can find a corresponding
error class in Spark, we should use the correct error class. However, some of
the errors are more iceberg s
szehon-ho commented on PR #10766:
URL: https://github.com/apache/iceberg/pull/10766#issuecomment-2246856254
Hi @huaxingao thanks, this mostly makes sense to me, but just wanted to get
the motivation. Is it so the user can see the error comes from Iceberg vs
other part of Spark? Also, is t
huaxingao commented on PR #10766:
URL: https://github.com/apache/iceberg/pull/10766#issuecomment-2246758141
cc @szehon-ho
--
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.
T
huaxingao opened a new pull request, #10766:
URL: https://github.com/apache/iceberg/pull/10766
In Iceberg's `spark-extensions`, if we encounter any invalid SQL statements,
we throw Spark's `AnalysisException`. However, it would be better to have
Iceberg's own `IcebergAnalysisException` to d
20 matches
Mail list logo
