tanmayrauth commented on code in PR #1398:
URL: https://github.com/apache/iceberg-go/pull/1398#discussion_r3525748404
##########
table/conflict_validation.go:
##########
@@ -96,19 +96,23 @@ const (
WriteUpdateIsolationLevelDefault = IsolationSerializable
)
+// ErrInvalidIsolationLevel is returned when a configured isolation level
+// value is invalid.
+var ErrInvalidIsolationLevel = errors.New("invalid isolation level")
+
// readIsolationLevel returns the isolation level for the given
-// property key, falling back to defVal for a missing or unrecognized
-// value.
-func readIsolationLevel(props iceberg.Properties, key string, defVal
IsolationLevel) IsolationLevel {
+// property key, falling back to defVal when the key is absent.
+func readIsolationLevel(props iceberg.Properties, key string, defVal
IsolationLevel) (IsolationLevel, error) {
v, ok := props[key]
if !ok {
- return defVal
+ return defVal, nil
}
+
switch IsolationLevel(v) {
case IsolationSerializable, IsolationSnapshot:
- return IsolationLevel(v)
+ return IsolationLevel(v), nil
default:
- return defVal
+ return defVal, fmt.Errorf("%w: %q for property %q",
ErrInvalidIsolationLevel, v, key)
Review Comment:
This rejects anything that isn't exactly lowercase "serializable"/"snapshot"
(empty string included). I think Java's fromName is case-insensitive — if so, a
config ported from Java/Spark with "SERIALIZABLE" would now fail here. Worth
confirming; lowercasing v before the switch would keep parity.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
