paul-rogers commented on code in PR #13165: URL: https://github.com/apache/druid/pull/13165#discussion_r1014435696
########## server/src/main/java/org/apache/druid/catalog/model/CatalogUtils.java: ########## @@ -0,0 +1,147 @@ +/* + * 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.druid.catalog.model; + +import com.fasterxml.jackson.core.JsonProcessingException; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.google.common.base.Strings; +import org.apache.druid.java.util.common.IAE; +import org.apache.druid.java.util.common.ISE; +import org.apache.druid.java.util.common.StringUtils; +import org.apache.druid.java.util.common.granularity.Granularities; +import org.apache.druid.java.util.common.granularity.Granularity; +import org.apache.druid.java.util.common.granularity.PeriodGranularity; +import org.joda.time.Period; + +import javax.annotation.Nullable; + +import java.util.Arrays; +import java.util.Collection; +import java.util.List; +import java.util.Map; +import java.util.Objects; +import java.util.stream.Collectors; +import java.util.stream.Stream; + +public class CatalogUtils +{ + public static List<String> columnNames(List<ColumnSpec> columns) + { + return columns + .stream() + .map(col -> col.name()) + .collect(Collectors.toList()); + } + + /** + * Convert a catalog granularity string to the Druid form. Catalog granularities + * are either the usual descriptive strings (in any case), or an ISO period. + * For the odd interval, the interval name is also accepted (for the other + * intervals, the interval name is the descriptive string). + */ + public static Granularity asDruidGranularity(String value) + { + if (Strings.isNullOrEmpty(value)) { + return Granularities.ALL; + } + try { + return new PeriodGranularity(new Period(value), null, null); + } + catch (IllegalArgumentException e) { + throw new IAE(StringUtils.format("%s is an invalid period string", value)); + } + } + + /** + * {@code String}-to-{@code List<String>} conversion. The string can contain zero items, + * one items, or a list. The list items are separated by a comma and optional + * whitespace. + */ + public static List<String> stringToList(String value) + { + if (value == null) { + return null; + } + return Arrays.asList(value.split(",\\s*")); + } + + public static <T> T safeCast(Object value, Class<T> type, String propertyName) + { + if (value == null) { + return null; + } + try { + return type.cast(value); + } + catch (ClassCastException e) { + throw new IAE("Value [%s] is not valid for property %s, expected type %s", + value, + propertyName, + type.getSimpleName() + ); + } + } + + public static <T> T safeGet(Map<String, Object> map, String propertyName, Class<T> type) + { + return safeCast(map.get(propertyName), type, propertyName); + } + + public static String stringListToLines(List<String> lines) + { + if (lines.isEmpty()) { + return ""; + } + return String.join("\n", lines) + "\n"; + } + + /** + * Catalog-specific 1uick & easy implementation of {@code toString()} for objects + * which are primarily representations of JSON objects. Use only for cases where the + * {@code toString()} is for debugging: the cost of creating an object mapper + * every time is undesirable for production code. Also, assumes that the + * type can serialized using the default mapper: doesn't work for types that + * require custom Jackson extensions. The catalog, however, has a simple type + * hierarchy, which is not extended via extensions, and so the default object mapper is + * fine. + */ + public static String toString(Object obj) + { + ObjectMapper jsonMapper = new ObjectMapper(); Review Comment: This method has been quite controversial! `toString()` is called only when debugging: no production code calls it. Since the purpose of this object is to be serialized to/from JSON, we avoid the need to have to hand-main a JSON-like to-string implementation that we use in many places. Instead, we just convert the object to JSON and display that. The catalog JSON objects are super simple: they can be converted with the default settings. That may not always be true. When it is no longer true, then we'll convert each method that calls this utility method to hand-code its to string (and try to remember to keep it up to date.) For now, this is a quick & dirty solution that aids debugging. ########## server/src/main/java/org/apache/druid/catalog/model/ModelProperties.java: ########## @@ -0,0 +1,305 @@ +/* + * 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.druid.catalog.model; + +import com.fasterxml.jackson.core.type.TypeReference; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.google.common.base.Preconditions; +import org.apache.druid.java.util.common.IAE; +import org.apache.druid.java.util.common.StringUtils; +import org.apache.druid.java.util.common.granularity.PeriodGranularity; +import org.joda.time.Period; + +import java.util.ArrayList; +import java.util.HashSet; +import java.util.List; +import java.util.Set; + +/** + * Definition of a top-level property in a catalog object. + * Provides a set of typical property definitions. Others can be + * created case-by-case. + * <p> + * Property definitions define the property name, validate the value, + * and merge updates. Properties have a type: but the type is implicit + * via the validation, as is needed when the type is actually a map + * which represents a Java object, or when the value is a list. + */ +public interface ModelProperties +{ + interface PropertyDefn<T> + { + String name(); + String typeName(); + void validate(Object value, ObjectMapper jsonMapper); + Object merge(Object existing, Object update); Review Comment: Technically, yes. But, note that the incoming value is `Object` and the implementation is often: ```java return update == null ? existing : update; ``` This is only ever used to merge maps (where the values are generic), so there seemed nothing to e gained from all the unnecessary casting required to cast to type `T`. ########## server/src/main/java/org/apache/druid/catalog/model/Columns.java: ########## @@ -0,0 +1,105 @@ +/* + * 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.druid.catalog.model; + +import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableSet; +import org.apache.druid.java.util.common.IAE; +import org.apache.druid.java.util.common.StringUtils; +import org.apache.druid.segment.column.ColumnType; +import org.apache.druid.segment.column.RowSignature; + +import java.util.List; +import java.util.Map; +import java.util.Set; + +public class Columns +{ + public static final String TIME_COLUMN = "__time"; + + public static final String VARCHAR = "VARCHAR"; + public static final String BIGINT = "BIGINT"; + public static final String FLOAT = "FLOAT"; + public static final String DOUBLE = "DOUBLE"; + public static final String TIMESTAMP = "TIMESTAMP"; + + public static final Set<String> NUMERIC_TYPES = ImmutableSet.of(BIGINT, FLOAT, DOUBLE); Review Comment: Yet... ########## server/src/main/java/org/apache/druid/catalog/model/table/HttpTableDefn.java: ########## @@ -0,0 +1,190 @@ +/* + * 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.druid.catalog.model.table; + +import com.google.common.base.Strings; +import com.google.common.collect.ImmutableMap; +import org.apache.druid.catalog.model.CatalogUtils; +import org.apache.druid.catalog.model.ModelProperties.StringListPropertyDefn; +import org.apache.druid.catalog.model.ModelProperties.StringPropertyDefn; +import org.apache.druid.catalog.model.ParameterizedDefn; +import org.apache.druid.catalog.model.ResolvedTable; +import org.apache.druid.catalog.model.table.ExternalTableDefn.FormattedExternalTableDefn; +import org.apache.druid.data.input.InputSource; +import org.apache.druid.data.input.impl.HttpInputSource; +import org.apache.druid.java.util.common.IAE; +import org.apache.druid.java.util.common.ISE; +import org.apache.druid.java.util.common.StringUtils; +import org.apache.druid.metadata.DefaultPasswordProvider; +import org.apache.druid.metadata.EnvironmentVariablePasswordProvider; +import org.apache.druid.utils.CollectionUtils; + +import java.net.URI; +import java.net.URISyntaxException; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +/** + * Definition of an input table for an HTTP data source. Provides the same + * properties as the {@link HttpInputSource}, but as top-level properties + * that can be mapped to SQL function parameters. Property names are + * cleaned up for ease-of-use. The HTTP input source has multiple quirks, + * the conversion method smooths over those quirks for a simpler catalog + * experience. Provides a parameterized + * form where the user provides the partial URLs to use for a particular + * query. + */ +public class HttpTableDefn extends FormattedExternalTableDefn implements ParameterizedDefn +{ + public static final String TABLE_TYPE = HttpInputSource.TYPE_KEY; + public static final String URI_TEMPLATE_PROPERTY = "template"; Review Comment: I rather expected to provide that info in the docs. It already exists in the issue describing the feature. Us developer-types get the correct, up-to-date info by looking at the code. ########## server/src/main/java/org/apache/druid/catalog/model/table/HttpTableDefn.java: ########## @@ -0,0 +1,190 @@ +/* + * 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.druid.catalog.model.table; + +import com.google.common.base.Strings; +import com.google.common.collect.ImmutableMap; +import org.apache.druid.catalog.model.CatalogUtils; +import org.apache.druid.catalog.model.ModelProperties.StringListPropertyDefn; +import org.apache.druid.catalog.model.ModelProperties.StringPropertyDefn; +import org.apache.druid.catalog.model.ParameterizedDefn; +import org.apache.druid.catalog.model.ResolvedTable; +import org.apache.druid.catalog.model.table.ExternalTableDefn.FormattedExternalTableDefn; +import org.apache.druid.data.input.InputSource; +import org.apache.druid.data.input.impl.HttpInputSource; +import org.apache.druid.java.util.common.IAE; +import org.apache.druid.java.util.common.ISE; +import org.apache.druid.java.util.common.StringUtils; +import org.apache.druid.metadata.DefaultPasswordProvider; +import org.apache.druid.metadata.EnvironmentVariablePasswordProvider; +import org.apache.druid.utils.CollectionUtils; + +import java.net.URI; +import java.net.URISyntaxException; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +/** + * Definition of an input table for an HTTP data source. Provides the same + * properties as the {@link HttpInputSource}, but as top-level properties + * that can be mapped to SQL function parameters. Property names are + * cleaned up for ease-of-use. The HTTP input source has multiple quirks, + * the conversion method smooths over those quirks for a simpler catalog + * experience. Provides a parameterized + * form where the user provides the partial URLs to use for a particular + * query. + */ +public class HttpTableDefn extends FormattedExternalTableDefn implements ParameterizedDefn +{ + public static final String TABLE_TYPE = HttpInputSource.TYPE_KEY; + public static final String URI_TEMPLATE_PROPERTY = "template"; Review Comment: This is one of those context questions. To you, reading this file amid zillions of others, the term "template" is too generic. To a user, focused on the HTTP input source, there is only one thing that could be templated: the URL. Since these values also appear in SQL, shorter names is a convenience for the user: ```SQL ... FROM (TABLE(http(template => 'http://foo.bar/{}.html'))) ``` vs. ```SQL ... FROM (TABLE(http(httpInputSourceURLTemplate => 'http://foo.bar/{}.html'))) ``` That said, changed it to `uriTemplate`. Three more letters to type and a shift key to hit... ########## server/src/main/java/org/apache/druid/catalog/model/TableId.java: ########## @@ -0,0 +1,113 @@ +/* + * 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.druid.catalog.model; + +import com.fasterxml.jackson.annotation.JsonCreator; +import com.fasterxml.jackson.annotation.JsonProperty; +import com.google.common.base.Objects; +import org.apache.druid.java.util.common.StringUtils; + +/** + * SQL-like compound table ID with schema and table name. + */ +public class TableId +{ + // Well-known Druid schemas + public static final String DRUID_SCHEMA = "druid"; + public static final String LOOKUP_SCHEMA = "lookups"; + public static final String SYSTEM_SCHEMA = "sys"; + public static final String CATALOG_SCHEMA = "INFORMATION_SCHEMA"; + + // Extra for MSQE + public static final String EXTERNAL_SCHEMA = "extern"; + + // Extra for views + public static final String VIEW_SCHEMA = "view"; + + private final String schema; + private final String name; + + @JsonCreator + public TableId( + @JsonProperty("schema") String schema, Review Comment: This is a simple holder. The checks are done elsewhere. See discussion in an earlier note about providing good error messages: that is best done at the level that knows the user's intent in providing the names. ("A table name is required in order to edit a table.") say. ########## server/src/main/java/org/apache/druid/catalog/model/ObjectDefn.java: ########## @@ -0,0 +1,138 @@ +/* + * 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.druid.catalog.model; + +import com.fasterxml.jackson.databind.ObjectMapper; +import com.google.common.collect.ImmutableMap; +import org.apache.druid.catalog.model.ModelProperties.PropertyDefn; + +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +/** + * Metadata definition of the metadata objects stored in the catalog. (Yes, + * that means that this is meta-meta-data.) Objects consist of a map of + * property values (and perhaps other items defined in subclasses.) Each + * property is defined by a column metadata object. Objects allow extended + * properties which have no definition: the meaning of such properties is + * defined elsewhere. + */ +public class ObjectDefn +{ + private final String name; + private final String typeValue; + private final Map<String, PropertyDefn<?>> properties; + + public ObjectDefn( + final String name, + final String typeValue, + final List<PropertyDefn<?>> fields + ) + { + this.name = name; + this.typeValue = typeValue; + this.properties = toPropertyMap(fields); + } + + protected static Map<String, PropertyDefn<?>> toPropertyMap(final List<PropertyDefn<?>> props) + { + ImmutableMap.Builder<String, PropertyDefn<?>> builder = ImmutableMap.builder(); + if (props != null) { + for (PropertyDefn<?> prop : props) { + builder.put(prop.name(), prop); + } + } + return builder.build(); + } + + public String name() + { + return name; + } + + /** + * The type value is the value of the {@code "type"} field written into the + * object's Java or JSON representation. It is akin to the type used by + * Jackson. + */ + public String typeValue() + { + return typeValue; + } + + public Map<String, PropertyDefn<?>> properties() + { + return properties; + } + + public PropertyDefn<?> property(String key) + { + return properties.get(key); + } + + /** + * Merge the properties for an object using a set of updates in a map. If the + * update value is {@code null}, then remove the property in the revised set. If the + * property is known, use the column definition to merge the values. Else, the + * update replaces any existing value. + * <p> + * This method does not validate the properties, except as needed to do a Review Comment: There are two reasons. One is simplicity: might as well do the merge (only) then validate (only) since we need to do the validate for brand new objects (without the merge), and we only want to write the code once. The second reason is that properties might be inter-dependent. Imagine we stored the rollup grain for a table with rollup. In that case, a good rule is that the rollup grain can be no greater than the storage grain. That validation requires looking at two properties. If we did it here, we'd have a race condition: the order that the properties appear in the update would determine whether the update is valid. Such a restriction would not, in general, make the user's life easier. ########## server/src/main/java/org/apache/druid/catalog/model/ParameterizedDefn.java: ########## @@ -0,0 +1,62 @@ +/* + * 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.druid.catalog.model; + +import org.apache.druid.catalog.model.table.ExternalTableSpec; + +import java.util.List; +import java.util.Map; + +public interface ParameterizedDefn Review Comment: Done. The purpose of this will be come clearer once the Calcite integration is available. ########## server/src/main/java/org/apache/druid/catalog/model/TableDefnRegistry.java: ########## @@ -0,0 +1,87 @@ +/* + * 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.druid.catalog.model; + +import com.fasterxml.jackson.databind.ObjectMapper; +import com.google.common.base.Strings; +import com.google.common.collect.ImmutableMap; +import org.apache.druid.catalog.model.table.DatasourceDefn; +import org.apache.druid.catalog.model.table.HttpTableDefn; +import org.apache.druid.catalog.model.table.InlineTableDefn; +import org.apache.druid.catalog.model.table.LocalTableDefn; +import org.apache.druid.java.util.common.IAE; + +import java.util.Map; + +public class TableDefnRegistry +{ + private static final TableDefn[] TABLE_DEFNS = { + new DatasourceDefn(), + new InlineTableDefn(), + new HttpTableDefn(), + new LocalTableDefn() + }; + + private final Map<String, TableDefn> defns; + private final ObjectMapper jsonMapper; + + public TableDefnRegistry( Review Comment: Why? In this early version, we hand-create an instance. Later, we'll configure Guice to do the deed. Either way, we need to create instances. ########## extensions-core/druid-catalog/src/main/java/org/apache/druid/catalog/sync/CachedMetadataCatalog.java: ########## @@ -84,14 +97,14 @@ protected TableEntry(TableMetadata table) { private final SchemaSpec schema; private long version = NOT_FETCHED; - private final ConcurrentHashMap<String, TableEntry> cache = new ConcurrentHashMap<>(); + private final Map<String, TableEntry> cache = new TreeMap<>(); Review Comment: Reverted this change. Explained that the method that gets the sorted list is low frequency. Once we do the Calcite integration, we may find we don't even need it since that layer has recently been changed to no longer ask for a list of all tables. ########## server/src/main/java/org/apache/druid/catalog/model/Columns.java: ########## @@ -0,0 +1,105 @@ +/* + * 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.druid.catalog.model; + +import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableSet; +import org.apache.druid.java.util.common.IAE; +import org.apache.druid.java.util.common.StringUtils; +import org.apache.druid.segment.column.ColumnType; +import org.apache.druid.segment.column.RowSignature; + +import java.util.List; +import java.util.Map; +import java.util.Set; + +public class Columns Review Comment: Sure. That's kind of old-school. You can create instances if you want, but the compiler will complain if you do anything with it since you'll be accessing static methods in a non-static way. ########## server/src/main/java/org/apache/druid/catalog/model/ColumnDefn.java: ########## @@ -0,0 +1,110 @@ +/* + * 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.druid.catalog.model; + +import com.fasterxml.jackson.databind.ObjectMapper; +import org.apache.druid.catalog.model.ModelProperties.PropertyDefn; +import org.apache.druid.java.util.common.IAE; +import org.apache.druid.java.util.common.StringUtils; + +import java.util.List; +import java.util.Map; + +public class ColumnDefn extends ObjectDefn +{ + /** + * Convenience class that holds a column specification and its corresponding + * definition. This allows the spec to be a pure "data object" without knowledge + * of the metadata representation given by the column definition. + */ + public static class ResolvedColumn + { + private final ColumnDefn defn; + private final ColumnSpec spec; + + public ResolvedColumn(ColumnDefn defn, ColumnSpec spec) + { + this.defn = defn; + this.spec = spec; + } + + public ColumnDefn defn() Review Comment: Yet... The resolved column is a "smart" pair of (defn, spec). So, access to the definition is presumed to be useful as the project proceeds. ########## server/src/main/java/org/apache/druid/catalog/model/table/ClusterKeySpec.java: ########## @@ -0,0 +1,83 @@ +/* + * 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.druid.catalog.model.table; + +import com.fasterxml.jackson.annotation.JsonCreator; +import com.fasterxml.jackson.annotation.JsonProperty; + +import javax.annotation.Nullable; + +import java.util.Objects; + +public class ClusterKeySpec +{ + private final String expr; + private final boolean desc; Review Comment: One other thought. Note that the asc/desc option is designed to default to the most frequent choice: ```java public ClusterKeySpec( @JsonProperty("column") String expr, @JsonProperty("desc") @Nullable Boolean desc ) { this.expr = expr; this.desc = desc != null && desc; } ``` Let's say we wanted to be SQL compliant and let the user specify `NULLS HIGH` or `NULLS LOW`. We would use the same trick which would be nicely backward compatible: ```java public ClusterKeySpec( @JsonProperty("column") String expr, @JsonProperty("desc") @Nullable Boolean desc, @JsonProperty("nullsHigh") @Nullable Boolean nullsHigh ) { this.expr = expr; this.desc = desc != null && desc; this.nullsHigh = nullsHigh != null & nullsHigh; } ``` ########## server/src/main/java/org/apache/druid/catalog/model/ColumnDefn.java: ########## @@ -0,0 +1,110 @@ +/* + * 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.druid.catalog.model; + +import com.fasterxml.jackson.databind.ObjectMapper; +import org.apache.druid.catalog.model.ModelProperties.PropertyDefn; +import org.apache.druid.java.util.common.IAE; +import org.apache.druid.java.util.common.StringUtils; + +import java.util.List; +import java.util.Map; + +public class ColumnDefn extends ObjectDefn +{ + /** + * Convenience class that holds a column specification and its corresponding + * definition. This allows the spec to be a pure "data object" without knowledge + * of the metadata representation given by the column definition. + */ + public static class ResolvedColumn + { + private final ColumnDefn defn; + private final ColumnSpec spec; + + public ResolvedColumn(ColumnDefn defn, ColumnSpec spec) + { + this.defn = defn; + this.spec = spec; + } + + public ColumnDefn defn() + { + return defn; + } + + public ColumnSpec spec() + { + return spec; + } + + public ResolvedColumn merge(ColumnSpec update) + { + return new ResolvedColumn(defn, defn.merge(spec, update)); + } + + public void validate(ObjectMapper jsonMapper) + { + defn.validate(spec, jsonMapper); + } + } + + public ColumnDefn( + final String name, + final String typeValue, + final List<PropertyDefn<?>> fields + ) + { + super(name, typeValue, fields); + } + + public ColumnSpec merge(ColumnSpec spec, ColumnSpec update) + { + String updateType = update.type(); + if (updateType != null && !spec.type().equals(updateType)) { + throw new IAE("The update type must be null or [%s]", spec.type()); + } + String revisedType = update.sqlType() == null ? spec.sqlType() : update.sqlType(); + Map<String, Object> revisedProps = mergeProperties( + spec.properties(), + update.properties() + ); + return new ColumnSpec(spec.type(), spec.name(), revisedType, revisedProps); + } + + public void validate(ColumnSpec spec, ObjectMapper jsonMapper) + { + spec.validate(); + } + + public void validateScalarColumn(ColumnSpec spec) + { + Columns.validateScalarColumn(spec.name(), spec.sqlType()); + if (Columns.isTimeColumn(spec.name())) { Review Comment: Ah, my bad. The base version should not do this check: we can't prevent you from having a `__time` column in a CSV file that is whatever type you want. Moved the check to the datasource column definition. ########## server/src/main/java/org/apache/druid/catalog/model/table/HttpTableDefn.java: ########## @@ -0,0 +1,190 @@ +/* + * 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.druid.catalog.model.table; + +import com.google.common.base.Strings; +import com.google.common.collect.ImmutableMap; +import org.apache.druid.catalog.model.CatalogUtils; +import org.apache.druid.catalog.model.ModelProperties.StringListPropertyDefn; +import org.apache.druid.catalog.model.ModelProperties.StringPropertyDefn; +import org.apache.druid.catalog.model.ParameterizedDefn; +import org.apache.druid.catalog.model.ResolvedTable; +import org.apache.druid.catalog.model.table.ExternalTableDefn.FormattedExternalTableDefn; +import org.apache.druid.data.input.InputSource; +import org.apache.druid.data.input.impl.HttpInputSource; +import org.apache.druid.java.util.common.IAE; +import org.apache.druid.java.util.common.ISE; +import org.apache.druid.java.util.common.StringUtils; +import org.apache.druid.metadata.DefaultPasswordProvider; +import org.apache.druid.metadata.EnvironmentVariablePasswordProvider; +import org.apache.druid.utils.CollectionUtils; + +import java.net.URI; +import java.net.URISyntaxException; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +/** + * Definition of an input table for an HTTP data source. Provides the same + * properties as the {@link HttpInputSource}, but as top-level properties + * that can be mapped to SQL function parameters. Property names are + * cleaned up for ease-of-use. The HTTP input source has multiple quirks, + * the conversion method smooths over those quirks for a simpler catalog + * experience. Provides a parameterized + * form where the user provides the partial URLs to use for a particular + * query. + */ +public class HttpTableDefn extends FormattedExternalTableDefn implements ParameterizedDefn +{ + public static final String TABLE_TYPE = HttpInputSource.TYPE_KEY; + public static final String URI_TEMPLATE_PROPERTY = "template"; + public static final String USER_PROPERTY = "user"; + public static final String PASSWORD_PROPERTY = "password"; + public static final String PASSWORD_ENV_VAR_PROPERTY = "passwordEnvVar"; + public static final String URIS_PROPERTY = "uris"; + public static final String URIS_PARAMETER = "uris"; Review Comment: Because it is? I can specify it _either_ in the catalog (property) or in a SQL table statement (parameter). Not all properties are parameters and visa-versa. In this case, `uris` is both. ########## server/src/main/java/org/apache/druid/catalog/model/TableDefn.java: ########## @@ -0,0 +1,176 @@ +/* + * 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.druid.catalog.model; + +import com.fasterxml.jackson.databind.ObjectMapper; +import com.google.common.base.Strings; +import com.google.common.collect.ImmutableMap; +import org.apache.druid.catalog.model.ModelProperties.PropertyDefn; +import org.apache.druid.java.util.common.IAE; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.HashMap; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; + +/** + * Definition for all tables in the catalog. All tables have both + * properties and a schema. Subclasses define specific table types + * such as datasources or input tables. Some tables may be parameterized + * to allow the table to appear in a SQL table function by implementing + * the {@link ParameterizedDefn} interface. + */ +public class TableDefn extends ObjectDefn +{ + /** + * Human-readable description of the datasource. + */ + public static final String DESCRIPTION_PROPERTY = "description"; + + private final Map<String, ColumnDefn> columnDefns; + + public TableDefn( + final String name, + final String typeValue, + final List<PropertyDefn<?>> properties, + final List<ColumnDefn> columnDefns + ) + { + super( + name, + typeValue, + CatalogUtils.concatLists( + Collections.singletonList( + new ModelProperties.StringPropertyDefn(DESCRIPTION_PROPERTY) + ), + properties + ) + ); + this.columnDefns = columnDefns == null ? Collections.emptyMap() : toColumnMap(columnDefns); + } + + public static Map<String, ColumnDefn> toColumnMap(final List<ColumnDefn> colTypes) + { + ImmutableMap.Builder<String, ColumnDefn> builder = ImmutableMap.builder(); + for (ColumnDefn colType : colTypes) { + builder.put(colType.typeValue(), colType); + } + return builder.build(); + } + + /** + * Validate a table spec using the table, field and column definitions defined + * here. The column definitions validate the type of each property value using + * the object mapper. + */ + public void validate(ResolvedTable table) + { + validate(table.properties(), table.jsonMapper()); + validateColumns(table.spec().columns(), table.jsonMapper()); + } + + public void validateColumns(List<ColumnSpec> columns, ObjectMapper jsonMapper) + { + if (columns == null) { + return; + } + Set<String> names = new HashSet<>(); + for (ColumnSpec colSpec : columns) { + if (!names.add(colSpec.name())) { + throw new IAE("Duplicate column name: " + colSpec.name()); + } + ColumnDefn.ResolvedColumn resolvedCol = resolveColumn(colSpec); + resolvedCol.validate(jsonMapper); + } + } + + /** + * Resolve the column type to produce a composite object that holds + * both the definition and the column spec. + */ + public ColumnDefn.ResolvedColumn resolveColumn(ColumnSpec spec) + { + String type = spec.type(); + if (Strings.isNullOrEmpty(type)) { + throw new IAE("The column type is required."); + } + ColumnDefn defn = columnDefns.get(type); + if (defn == null) { + throw new IAE("Column type [%s] is not valid for tables of type [%s].", type, typeValue()); + } + return new ColumnDefn.ResolvedColumn(defn, spec); + } + + /** + * Merge a table spec with an update. The merge affects both the properties and + * the list of columns. + */ + public TableSpec merge(TableSpec spec, TableSpec update, ObjectMapper jsonMapper) + { + String updateType = update.type(); + if (updateType != null && !spec.type().equals(updateType)) { + throw new IAE("The update type must be null or [%s]", spec.type()); + } + Map<String, Object> revisedProps = mergeProperties(spec.properties(), update.properties()); + List<ColumnSpec> revisedColumns = mergeColumns(spec.columns(), update.columns()); + TableSpec revisedSpec = new TableSpec(spec.type(), revisedProps, revisedColumns); + validate(new ResolvedTable(this, revisedSpec, jsonMapper)); + return revisedSpec; + } + + /** + * Merge the set of columns from an existing spec and an update. + * Columns are matched by name. If the column exists, then it is updated. If + * the column does not exist, then the new column is appended to the existing + * list. This merge operation cannot remove columns or change order. Review Comment: There is no good way to use a column spec to ask for its own removal. One way would be to include a "removeMe" property, but that seems hokey. Also, because the list of columns is a subset of the full list, there is no reasonable way to use that sublist to reorder the full list. If I have columns `(a, b, c, d, e, f)` in my table, but include updates for `(b, d)`, does that mean I want to move `d` to follow `b`? To move `b` to precede `d`? To move both to the start of the list? Or, do I want to remove `a`, `c`, `e` and `f`? If the latter, then an update is really just a replace, but we already have a replace operation, and the only way to update any one column would be to include all of them, which pretty much invalidates the idea of a focused update. Because of these ambiguities, we provide separate "edit" operations to remove or move columns. That way, the semantics are very clear. ########## server/src/main/java/org/apache/druid/catalog/model/table/ClusterKeySpec.java: ########## @@ -0,0 +1,83 @@ +/* + * 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.druid.catalog.model.table; + +import com.fasterxml.jackson.annotation.JsonCreator; +import com.fasterxml.jackson.annotation.JsonProperty; + +import javax.annotation.Nullable; + +import java.util.Objects; + +public class ClusterKeySpec +{ + private final String expr; + private final boolean desc; Review Comment: Exactly what would the extension be? Clustering is sorting. Sorting is either ascending or descending. Let's say we had a hash index. Could we have a "hash clustering"? I'm not sure what that means. If we want to extend, then let's wait for the new innovation. Then, if we, say, provide a hash table as a an alternative to sorting, we provide a `hashing` property that defines the attributes of that alternative, and say that you can provide either clustering or ordering. If there is something in the works now, then pease point me to the spec and I'll be happy to model it in the catalog. -- 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]
