AMashenkov commented on code in PR #1831:
URL: https://github.com/apache/ignite-3/pull/1831#discussion_r1147417249
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/property/PropertiesHelper.java:
##########
@@ -20,14 +20,70 @@
import java.util.HashMap;
import java.util.List;
import java.util.Map;
+import org.apache.ignite.internal.sql.engine.property.PropertiesHolder.Builder;
import org.apache.ignite.internal.util.IgniteUtils;
/**
* Utility class to work with {@link Property}.
*/
public final class PropertiesHelper {
- private PropertiesHelper() {}
+ private PropertiesHelper() {
+ throw new IllegalStateException();
+ }
+
+ /** Creates empty holder. */
+ public static PropertiesHolder emptyHolder() {
+ return new PropertiesHolderImpl(Map.of());
+ }
+
+ /** Creates new builder. */
+ public static Builder newBuilder() {
+ return new BuilderImpl();
+ }
+
+ /**
+ * Merges two holder into single one.
+ *
+ * <p>If conflict arises, conflicted values of secondary holder will be
overridden with values
+ * of primary holder.
+ *
+ * @param primaryHolder A holder whose values will be preserved.
+ * @param secondaryHolder A holder whose values will be overridden in case
of conflict.
+ * @return A holder containing properties from both holders.
+ */
+ public static PropertiesHolder merge(PropertiesHolder primaryHolder,
PropertiesHolder secondaryHolder) {
+ Builder builder = builderFromHolder(secondaryHolder);
Review Comment:
As I see how this method is used, 'secondaryHolder' contains defaults, and
'primaryHolder' contains user props.
'builderFromHolder' validates the default values, but not the user ones.
Is it expected behaviour?
Should we validate props in builder method and then assume that property
value type is correct as the only way to create PropertisHolder is using the
builder.
--
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]
