You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by GitBox <gi...@apache.org> on 2023/01/13 15:01:53 UTC
[GitHub] [iceberg] nastra commented on a diff in pull request #6559: Core: View core parser implementations
nastra commented on code in PR #6559:
URL: https://github.com/apache/iceberg/pull/6559#discussion_r1069530876
##########
core/src/main/java/org/apache/iceberg/view/ViewProperties.java:
##########
@@ -0,0 +1,41 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.iceberg.view;
+
+/** View properties that can be set during CREATE/REPLACE view or using updateProperties API. */
+public class ViewProperties {
+ public static final String COMMIT_NUM_RETRIES = "commit.retry.num-retries";
+ public static final int COMMIT_NUM_RETRIES_DEFAULT = 4;
+
+ public static final String COMMIT_MIN_RETRY_WAIT_MS = "commit.retry.min-wait-ms";
+ public static final int COMMIT_MIN_RETRY_WAIT_MS_DEFAULT = 100;
+
+ public static final String COMMIT_MAX_RETRY_WAIT_MS = "commit.retry.max-wait-ms";
+ public static final int COMMIT_MAX_RETRY_WAIT_MS_DEFAULT = 60000; // 1 minute
+
+ public static final String COMMIT_TOTAL_RETRY_TIME_MS = "commit.retry.total-timeout-ms";
+ public static final int COMMIT_TOTAL_RETRY_TIME_MS_DEFAULT = 1800000; // 30 minutes
+
+ public static final String VERSION_HISTORY_SIZE = "version.history.num-entries";
+ public static final int VERSION_HISTORY_SIZE_DEFAULT = 10;
+
+ public static final String TABLE_COMMENT = "comment";
Review Comment:
seems to be unused
##########
core/src/test/java/org/apache/iceberg/view/ParserTestBase.java:
##########
@@ -0,0 +1,57 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.iceberg.view;
+
+import java.io.IOException;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import org.apache.iceberg.util.JsonUtil;
+import org.apache.iceberg.util.TestJsonUtil;
+import org.junit.Assert;
+import org.junit.Test;
+
+public abstract class ParserTestBase<T> {
Review Comment:
I'm not sure whether it's worth adding this base class. Are we planning to introduce this to all the other test classes that test parsers?
##########
core/src/main/java/org/apache/iceberg/view/ViewProperties.java:
##########
@@ -0,0 +1,41 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.iceberg.view;
+
+/** View properties that can be set during CREATE/REPLACE view or using updateProperties API. */
+public class ViewProperties {
+ public static final String COMMIT_NUM_RETRIES = "commit.retry.num-retries";
+ public static final int COMMIT_NUM_RETRIES_DEFAULT = 4;
+
+ public static final String COMMIT_MIN_RETRY_WAIT_MS = "commit.retry.min-wait-ms";
+ public static final int COMMIT_MIN_RETRY_WAIT_MS_DEFAULT = 100;
+
+ public static final String COMMIT_MAX_RETRY_WAIT_MS = "commit.retry.max-wait-ms";
+ public static final int COMMIT_MAX_RETRY_WAIT_MS_DEFAULT = 60000; // 1 minute
+
+ public static final String COMMIT_TOTAL_RETRY_TIME_MS = "commit.retry.total-timeout-ms";
+ public static final int COMMIT_TOTAL_RETRY_TIME_MS_DEFAULT = 1800000; // 30 minutes
Review Comment:
do we need to have separate min/max/total retry settings for views or could we just use whatever we use for tables?
##########
core/src/test/java/org/apache/iceberg/view/TestViewMetadataParser.java:
##########
@@ -0,0 +1,82 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.iceberg.view;
+
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+import org.apache.iceberg.Schema;
+import org.apache.iceberg.relocated.com.google.common.collect.ImmutableList;
+import org.apache.iceberg.types.Types;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+@RunWith(Parameterized.class)
+public class TestViewMetadataParser extends ParserTestBase<ViewMetadata> {
Review Comment:
I believe this is missing a few tests around nullability/empty jsons and such, similar to what I mentioned in https://github.com/apache/iceberg/pull/6565
##########
core/src/main/java/org/apache/iceberg/view/BaseSQLViewRepresentation.java:
##########
@@ -0,0 +1,206 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.iceberg.view;
+
+import java.util.Collections;
+import java.util.List;
+import java.util.Objects;
+import org.apache.iceberg.catalog.Namespace;
+import org.apache.iceberg.relocated.com.google.common.base.Preconditions;
+
+public class BaseSQLViewRepresentation implements SQLViewRepresentation {
+
+ private final String query;
Review Comment:
all of these new base classes are adding quite a lot of overhead, since they require manual builder classes (and technically even tests for those builders). It would be better to switch all of those newly added classes to using [`@Immutable`](https://immutables.github.io/). The end result will be:
* much shorter code
* we'll get the builders for free
* no need to add null checks and such as the Immutable builder will derive what needs to be checked for nulls based on the API definition.
Just to give you an example of how much shorter the code can look, I've moved those classes to using `@Immutable` in https://github.com/apache/iceberg/commit/97bcd5030e2a37ef8691d4589c40427e2061a2b8. The diff is a bit difficult to read because lots of code has been removed, but the resulting [files(s)](https://github.com/apache/iceberg/tree/97bcd5030e2a37ef8691d4589c40427e2061a2b8/core/src/main/java/org/apache/iceberg/view) are much shorter, such as https://github.com/apache/iceberg/blob/97bcd5030e2a37ef8691d4589c40427e2061a2b8/core/src/main/java/org/apache/iceberg/view/BaseSQLViewRepresentation.java
##########
core/src/main/java/org/apache/iceberg/util/JsonUtil.java:
##########
@@ -283,6 +284,63 @@ public static void writeLongFieldIf(
}
}
+ @FunctionalInterface
+ public interface JsonWriter<T> {
Review Comment:
it's probably better to have a separate PR for `JsonUtil` + `TestJsonUtil` changes
##########
core/src/main/java/org/apache/iceberg/view/ViewRepresentationParser.java:
##########
@@ -0,0 +1,65 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.iceberg.view;
+
+import com.fasterxml.jackson.core.JsonGenerator;
+import com.fasterxml.jackson.databind.JsonNode;
+import java.io.IOException;
+import java.util.Locale;
+
+public class ViewRepresentationParser {
+
+ enum Field {
+ TYPE;
+
+ public String fieldName() {
+ return name().toLowerCase(Locale.ENGLISH);
+ }
+ }
+
+ public static void toJson(ViewRepresentation representation, JsonGenerator generator)
+ throws IOException {
+ switch (representation.type()) {
+ case SQL:
+ SQLViewRepresentationParser.toJson((SQLViewRepresentation) representation, generator);
+ break;
+
+ default:
+ throw new IllegalArgumentException(
+ String.format(
+ "Unknown view representation type '%s' to serialize", representation.type()));
+ }
+ }
+
+ public static ViewRepresentation fromJson(JsonNode node) {
+ String typeName = node.get(Field.TYPE.fieldName()).asText();
Review Comment:
this needs a null check for `node` + tests
##########
core/src/main/java/org/apache/iceberg/view/ViewRepresentationParser.java:
##########
@@ -0,0 +1,65 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.iceberg.view;
+
+import com.fasterxml.jackson.core.JsonGenerator;
+import com.fasterxml.jackson.databind.JsonNode;
+import java.io.IOException;
+import java.util.Locale;
+
+public class ViewRepresentationParser {
+
+ enum Field {
+ TYPE;
+
+ public String fieldName() {
+ return name().toLowerCase(Locale.ENGLISH);
+ }
+ }
+
+ public static void toJson(ViewRepresentation representation, JsonGenerator generator)
+ throws IOException {
+ switch (representation.type()) {
Review Comment:
I believe this needs a null check for `representation`
##########
core/src/main/java/org/apache/iceberg/view/ViewHistoryEntryParser.java:
##########
@@ -0,0 +1,45 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.iceberg.view;
+
+import com.fasterxml.jackson.core.JsonGenerator;
+import com.fasterxml.jackson.databind.JsonNode;
+import java.io.IOException;
+import org.apache.iceberg.util.JsonUtil;
+
+class ViewHistoryEntryParser {
+
+ // visible for testing
+ static final String VERSION_ID = "version-id";
+ static final String TIMESTAMP_MS = "timestamp-ms";
+
+ static void toJson(ViewHistoryEntry entry, JsonGenerator generator) throws IOException {
Review Comment:
needs a null check (similar to how we do it in other parsers)
##########
core/src/main/java/org/apache/iceberg/view/ViewHistoryEntryParser.java:
##########
@@ -0,0 +1,45 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.iceberg.view;
+
+import com.fasterxml.jackson.core.JsonGenerator;
+import com.fasterxml.jackson.databind.JsonNode;
+import java.io.IOException;
+import org.apache.iceberg.util.JsonUtil;
+
+class ViewHistoryEntryParser {
+
+ // visible for testing
+ static final String VERSION_ID = "version-id";
+ static final String TIMESTAMP_MS = "timestamp-ms";
+
+ static void toJson(ViewHistoryEntry entry, JsonGenerator generator) throws IOException {
+ generator.writeStartObject();
+ generator.writeNumberField(TIMESTAMP_MS, entry.timestampMillis());
+ generator.writeNumberField(VERSION_ID, entry.versionId());
+ generator.writeEndObject();
+ }
+
+ static ViewHistoryEntry fromJson(JsonNode node) {
+ return BaseViewHistoryEntry.of(
Review Comment:
needs a null check (similar to how we do it in other parsers)
##########
core/src/main/java/org/apache/iceberg/view/ViewVersionParser.java:
##########
@@ -0,0 +1,68 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.iceberg.view;
+
+import com.fasterxml.jackson.core.JsonGenerator;
+import com.fasterxml.jackson.databind.JsonNode;
+import java.io.IOException;
+import java.util.List;
+import java.util.Map;
+import org.apache.iceberg.relocated.com.google.common.base.Preconditions;
+import org.apache.iceberg.util.JsonUtil;
+
+class ViewVersionParser {
+
+ private static final String VERSION_ID = "version-id";
+ private static final String TIMESTAMP_MS = "timestamp-ms";
+ private static final String SUMMARY = "summary";
+ private static final String OPERATION = "operation";
+ private static final String REPRESENTATIONS = "representations";
+
+ static void toJson(ViewVersion version, JsonGenerator generator) throws IOException {
Review Comment:
fromJson + toJson needs some null checks
##########
core/src/main/java/org/apache/iceberg/view/ViewVersionParser.java:
##########
@@ -0,0 +1,68 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.iceberg.view;
+
+import com.fasterxml.jackson.core.JsonGenerator;
+import com.fasterxml.jackson.databind.JsonNode;
+import java.io.IOException;
+import java.util.List;
+import java.util.Map;
+import org.apache.iceberg.relocated.com.google.common.base.Preconditions;
+import org.apache.iceberg.util.JsonUtil;
+
+class ViewVersionParser {
+
+ private static final String VERSION_ID = "version-id";
+ private static final String TIMESTAMP_MS = "timestamp-ms";
+ private static final String SUMMARY = "summary";
+ private static final String OPERATION = "operation";
Review Comment:
seems to be unused
##########
core/src/main/java/org/apache/iceberg/view/ViewVersionParser.java:
##########
@@ -0,0 +1,68 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.iceberg.view;
+
+import com.fasterxml.jackson.core.JsonGenerator;
+import com.fasterxml.jackson.databind.JsonNode;
+import java.io.IOException;
+import java.util.List;
+import java.util.Map;
+import org.apache.iceberg.relocated.com.google.common.base.Preconditions;
+import org.apache.iceberg.util.JsonUtil;
+
+class ViewVersionParser {
+
+ private static final String VERSION_ID = "version-id";
+ private static final String TIMESTAMP_MS = "timestamp-ms";
+ private static final String SUMMARY = "summary";
+ private static final String OPERATION = "operation";
+ private static final String REPRESENTATIONS = "representations";
+
+ static void toJson(ViewVersion version, JsonGenerator generator) throws IOException {
+ generator.writeStartObject();
+
+ generator.writeNumberField(VERSION_ID, version.versionId());
+ generator.writeNumberField(TIMESTAMP_MS, version.timestampMillis());
+ JsonUtil.writeStringMap(SUMMARY, version.summary(), generator);
+ JsonUtil.writeObjectList(
+ REPRESENTATIONS, version.representations(), ViewRepresentationParser::toJson, generator);
+
+ generator.writeEndObject();
+ }
+
+ static ViewVersion fromJson(JsonNode node) {
+ Preconditions.checkArgument(
+ node.isObject(), "Cannot parse table version from a non-object: %s", node);
+
+ int versionId = JsonUtil.getInt(VERSION_ID, node);
+ long timestamp = JsonUtil.getLong(TIMESTAMP_MS, node);
+ Map<String, String> summary = JsonUtil.getStringMap(SUMMARY, node);
+ List<ViewRepresentation> representations =
+ JsonUtil.getObjectList(REPRESENTATIONS, node, ViewRepresentationParser::fromJson);
+
+ return BaseViewVersion.builder()
+ .versionId(versionId)
+ .timestampMillis(timestamp)
+ .summary(summary)
+ .representations(representations)
+ .build();
+ }
+
+ private ViewVersionParser() {}
Review Comment:
nit: same as above, would be good to move it to the top
##########
core/src/main/java/org/apache/iceberg/view/ViewRepresentationParser.java:
##########
@@ -0,0 +1,65 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.iceberg.view;
+
+import com.fasterxml.jackson.core.JsonGenerator;
+import com.fasterxml.jackson.databind.JsonNode;
+import java.io.IOException;
+import java.util.Locale;
+
+public class ViewRepresentationParser {
+
+ enum Field {
+ TYPE;
+
+ public String fieldName() {
+ return name().toLowerCase(Locale.ENGLISH);
+ }
+ }
+
+ public static void toJson(ViewRepresentation representation, JsonGenerator generator)
+ throws IOException {
+ switch (representation.type()) {
+ case SQL:
+ SQLViewRepresentationParser.toJson((SQLViewRepresentation) representation, generator);
+ break;
+
+ default:
+ throw new IllegalArgumentException(
+ String.format(
+ "Unknown view representation type '%s' to serialize", representation.type()));
+ }
+ }
+
+ public static ViewRepresentation fromJson(JsonNode node) {
+ String typeName = node.get(Field.TYPE.fieldName()).asText();
+ ViewRepresentation.Type type = ViewRepresentation.Type.fromString(typeName);
+
+ switch (type) {
+ case SQL:
+ return SQLViewRepresentationParser.fromJson(node);
+
+ default:
+ throw new IllegalStateException(
+ String.format("Unknown view representation type '%s' to deserialize", type));
+ }
+ }
+
+ private ViewRepresentationParser() {}
Review Comment:
nit: I think it would be better to move this to the top
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org