mihaibudiu commented on code in PR #4100:
URL: https://github.com/apache/calcite/pull/4100#discussion_r1941735033
##########
core/src/test/java/org/apache/calcite/test/JdbcTest.java:
##########
@@ -7489,7 +7488,7 @@ private void checkGetTimestamp(Connection con) throws
SQLException {
aSchema.setCacheEnabled(true);
// explicit should win implicit.
- assertThat(aSchema.getSubSchemaNames(), hasSize(1));
+ assertThat(aSchema.subSchemas().getNames(LikePattern.any()), hasSize(1));
Review Comment:
why change all these tests, can't the original function still be used?
##########
core/src/main/java/org/apache/calcite/schema/lookup/MappedLookup.java:
##########
@@ -0,0 +1,54 @@
+/*
+ * 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.calcite.schema.lookup;
+
+import org.checkerframework.checker.nullness.qual.Nullable;
+
+import java.util.Set;
+import java.util.function.BiFunction;
+
+/**
+ * A Lookup class which can be used to map different element types.
Review Comment:
map -> transform
using a supplied transform function.
I would rename "mapper" to "transform"
I think "map" is way too overloaded
##########
core/src/main/java/org/apache/calcite/adapter/jdbc/JdbcBaseSchema.java:
##########
@@ -0,0 +1,94 @@
+/*
+ * 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.calcite.adapter.jdbc;
+
+import org.apache.calcite.linq4j.tree.Expression;
+import org.apache.calcite.rel.type.RelProtoDataType;
+import org.apache.calcite.schema.Function;
+import org.apache.calcite.schema.Schema;
+import org.apache.calcite.schema.SchemaPlus;
+import org.apache.calcite.schema.SchemaVersion;
+import org.apache.calcite.schema.Schemas;
+import org.apache.calcite.schema.Table;
+import org.apache.calcite.schema.lookup.LikePattern;
+import org.apache.calcite.schema.lookup.Lookup;
+
+import org.checkerframework.checker.nullness.qual.Nullable;
+
+import java.util.Collection;
+import java.util.Collections;
+import java.util.Set;
+
+import static java.util.Objects.requireNonNull;
+
+/**
+ * Base class for JDBC schemas.
+ */
+public abstract class JdbcBaseSchema implements Schema {
+
+ @Override public abstract Lookup<Table> tables();
+
+
+ @Override public @Nullable Table getTable(String name) {
+ return tables().get(name);
+ }
+
+ @Override public Set<String> getTableNames() {
Review Comment:
can we move these functions as default methods in the base interface?
##########
core/src/test/java/org/apache/calcite/schema/lookup/ConcatLookupTest.java:
##########
@@ -0,0 +1,43 @@
+/*
+ * 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.calcite.schema.lookup;
+
+import org.junit.jupiter.api.Test;
+
+import static org.hamcrest.CoreMatchers.nullValue;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.equalTo;
+
+/**
+ * Test for ConcatLookup.
+ */
+class ConcatLookupTest {
+ private final Lookup<String> testee =
+ Lookup.concat(new MapLookup("a", "1"), new MapLookup("b", "2"));
+
+ @Test void testNull() {
+ assertThat(testee.get("c"), nullValue());
+ }
+
+ @Test void test() {
+ assertThat(testee.get("a"), equalTo("1"));
+ }
+
+ @Test void testIgnoreCase() {
Review Comment:
do you want to test the case of having names in common?
##########
core/src/main/java/org/apache/calcite/adapter/jdbc/JdbcCatalogSchema.java:
##########
@@ -103,34 +143,25 @@ public static JdbcCatalogSchema create(
return new JdbcCatalogSchema(dataSource, dialect, convention, catalog);
}
- private SubSchemaMap computeSubSchemaMap() {
- final ImmutableMap.Builder<String, Schema> builder =
- ImmutableMap.builder();
- @Nullable String defaultSchemaName;
- try (Connection connection = dataSource.getConnection();
- ResultSet resultSet =
- connection.getMetaData().getSchemas(catalog, null)) {
- defaultSchemaName = connection.getSchema();
- while (resultSet.next()) {
- final String schemaName =
- requireNonNull(resultSet.getString(1),
- "got null schemaName from the database");
- builder.put(schemaName,
- new JdbcSchema(dataSource, dialect, convention, catalog,
schemaName));
- }
+ @Override public Lookup<Table> tables() {
+ return Lookup.empty();
+ }
+
+ @Override public Lookup<? extends Schema> subSchemas() {
Review Comment:
what happens if the subschemas change in the underlying connection?
is there an invalidation protocol?
##########
core/src/main/java/org/apache/calcite/jdbc/CalciteSchema.java:
##########
@@ -335,14 +334,14 @@ public NavigableMap<String, LatticeEntry> getLatticeMap()
{
/** Returns the set of all table names. Includes implicit and explicit tables
* and functions with zero parameters. */
- public final NavigableSet<String> getTableNames() {
- final ImmutableSortedSet.Builder<String> builder =
- new ImmutableSortedSet.Builder<>(NameSet.COMPARATOR);
- // Add explicit tables, case-sensitive.
- builder.addAll(tableMap.map().keySet());
- // Add implicit tables, case-sensitive.
- addImplicitTableToBuilder(builder);
- return builder.build();
+ public final Set<String> getTableNames() {
+ return getTableNames(LikePattern.any());
+ }
+
+ /** Returns the set of filtered table names. Includes implicit and explicit
tables
Review Comment:
filtered by the pattern
##########
core/src/main/java/org/apache/calcite/adapter/jdbc/JdbcSchema.java:
##########
@@ -237,80 +252,86 @@ public DataSource getDataSource() {
return Schemas.subSchemaExpression(parentSchema, name, JdbcSchema.class);
}
- protected Multimap<String, Function> getFunctions() {
- // TODO: populate map from JDBC metadata
- return ImmutableMultimap.of();
+ private Stream<MetaImpl.MetaTable> getMetaTableStream(String
tableNamePattern) {
+ final Pair<@Nullable String, @Nullable String> catalogSchema =
getCatalogSchema();
+ final Stream<MetaImpl.MetaTable> tableDefs;
+ Connection connection = null;
+ ResultSet resultSet = null;
+ try {
+ connection = dataSource.getConnection();
+ final DatabaseMetaData metaData = connection.getMetaData();
+ resultSet =
+ metaData.getTables(catalogSchema.left, catalogSchema.right,
tableNamePattern, null);
+ tableDefs = asStream(connection, resultSet)
+ .map(JdbcSchema::metaDataMapper);
+ } catch (SQLException e) {
+ close(connection, null, resultSet);
+ throw new RuntimeException(
+ "Exception while reading tables", e);
+ }
+ return tableDefs;
}
- @Override public final Collection<Function> getFunctions(String name) {
- return getFunctions().get(name); // never null
+ private static Stream<ResultSet> asStream(Connection connection, ResultSet
resultSet) {
+ return StreamSupport.stream(
+ new Spliterators.AbstractSpliterator<ResultSet>(
+ Long.MAX_VALUE, Spliterator.ORDERED) {
+ @Override public boolean tryAdvance(Consumer<? super ResultSet>
action) {
+ try {
+ if (!resultSet.next()) {
+ return false;
+ }
+ action.accept(resultSet);
+ return true;
+ } catch (SQLException ex) {
+ throw new RuntimeException(ex);
+ }
+ }
+ }, false).onClose(() -> close(connection, null, resultSet));
}
- @Override public final Set<String> getFunctionNames() {
- return getFunctions().keySet();
+ private JdbcTable jdbcTableMapper(MetaImpl.MetaTable tableDef) {
+ return new JdbcTable(this, tableDef.tableCat, tableDef.tableSchem,
tableDef.tableName,
+ getTableType(tableDef.tableType));
}
- private ImmutableMap<String, JdbcTable> computeTables() {
- Connection connection = null;
- ResultSet resultSet = null;
+ private static MetaImpl.MetaTable metaDataMapper(ResultSet resultSet) {
try {
- connection = dataSource.getConnection();
- final Pair<@Nullable String, @Nullable String> catalogSchema =
getCatalogSchema(connection);
- final String catalog = catalogSchema.left;
- final String schema = catalogSchema.right;
- final Iterable<MetaImpl.MetaTable> tableDefs;
- Foo threadMetadata = THREAD_METADATA.get();
- if (threadMetadata != null) {
- tableDefs = threadMetadata.apply(catalog, schema);
- } else {
- final List<MetaImpl.MetaTable> tableDefList = new ArrayList<>();
- final DatabaseMetaData metaData = connection.getMetaData();
- resultSet = metaData.getTables(catalog, schema, null, null);
- while (resultSet.next()) {
- final String catalogName = resultSet.getString(1);
- final String schemaName = resultSet.getString(2);
- final String tableName = resultSet.getString(3);
- final String tableTypeName = resultSet.getString(4);
- tableDefList.add(
- new MetaImpl.MetaTable(catalogName, schemaName, tableName,
- tableTypeName));
- }
- tableDefs = tableDefList;
- }
+ return new MetaImpl.MetaTable(intern(resultSet.getString(1)),
intern(resultSet.getString(2)),
+ intern(resultSet.getString(3)),
+ intern(resultSet.getString(4)));
+ } catch (SQLException e) {
+ throw new RuntimeException(e);
+ }
+ }
- final ImmutableMap.Builder<String, JdbcTable> builder =
- ImmutableMap.builder();
- for (MetaImpl.MetaTable tableDef : tableDefs) {
- // Clean up table type. In particular, this ensures that 'SYSTEM
TABLE',
- // returned by Phoenix among others, maps to TableType.SYSTEM_TABLE.
- // We know enum constants are upper-case without spaces, so we can't
- // make things worse.
- //
- // PostgreSQL returns tableTypeName==null for pg_toast* tables
- // This can happen if you start JdbcSchema off a "public" PG schema
- // The tables are not designed to be queried by users, however we do
- // not filter them as we keep all the other table types.
- final String tableTypeName2 =
- tableDef.tableType == null
+ private static @Nullable String intern(@Nullable String string) {
+ if (string == null) {
+ return null;
+ }
+ return string.intern();
+ }
+
+ private static TableType getTableType(String tableTypeName) {
+ // Clean up table type. In particular, this ensures that 'SYSTEM TABLE',
+ // returned by Phoenix among others, maps to TableType.SYSTEM_TABLE.
+ // We know enum constants are upper-case without spaces, so we can't
+ // make things worse.
+ //
+ // PostgreSQL returns tableTypeName==null for pg_toast* tables
Review Comment:
I am not thrilled by this code - which seems inherited.
Ideally you would be able to check whether this is a Postgres database, and
only then do these "cleanups".
##########
core/src/main/java/org/apache/calcite/util/LazyReference.java:
##########
@@ -0,0 +1,47 @@
+/*
+ * 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.calcite.util;
+
+import java.util.concurrent.atomic.AtomicReference;
+import java.util.function.Supplier;
+
+/**
+ * This class can be used to lazily initialize an object.
+ *
+ * @param <T> Element Type
+ */
+public class LazyReference<T> {
+
+ private final AtomicReference<T> value = new AtomicReference<>();
+
+ public T getOrCompute(Supplier<T> supplier) {
Review Comment:
maybe the code is obvious, but I would still document the public API.
Since this is about concurrency, you should add in the comments a
description of what this promises with respect to concurrent accesses.
##########
core/src/main/java/org/apache/calcite/adapter/jdbc/JdbcBaseSchema.java:
##########
@@ -0,0 +1,94 @@
+/*
+ * 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.calcite.adapter.jdbc;
+
+import org.apache.calcite.linq4j.tree.Expression;
+import org.apache.calcite.rel.type.RelProtoDataType;
+import org.apache.calcite.schema.Function;
+import org.apache.calcite.schema.Schema;
+import org.apache.calcite.schema.SchemaPlus;
+import org.apache.calcite.schema.SchemaVersion;
+import org.apache.calcite.schema.Schemas;
+import org.apache.calcite.schema.Table;
+import org.apache.calcite.schema.lookup.LikePattern;
+import org.apache.calcite.schema.lookup.Lookup;
+
+import org.checkerframework.checker.nullness.qual.Nullable;
+
+import java.util.Collection;
+import java.util.Collections;
+import java.util.Set;
+
+import static java.util.Objects.requireNonNull;
+
+/**
+ * Base class for JDBC schemas.
+ */
+public abstract class JdbcBaseSchema implements Schema {
+
+ @Override public abstract Lookup<Table> tables();
+
+
+ @Override public @Nullable Table getTable(String name) {
+ return tables().get(name);
+ }
+
+ @Override public Set<String> getTableNames() {
+ return tables().getNames(LikePattern.any());
+ }
+
+ @Override public abstract Lookup<? extends Schema> subSchemas();
+
+ @Override public @Nullable Schema getSubSchema(String name) {
+ return subSchemas().get(name);
+ }
+
+ @Override public Set<String> getSubSchemaNames() {
+ return subSchemas().getNames(LikePattern.any());
+ }
+
+
+ @Override public @Nullable RelProtoDataType getType(String name) {
+ return null;
+ }
+
+ @Override public Set<String> getTypeNames() {
Review Comment:
do you expect these functions will ever return non-empty results?
(I don't know if JDBC is supposed to allow type lookups.)
##########
core/src/test/java/org/apache/calcite/schema/lookup/MapLookup.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.calcite.schema.lookup;
+
+import org.checkerframework.checker.nullness.qual.Nullable;
+
+import java.util.HashMap;
+import java.util.Locale;
+import java.util.Map;
+import java.util.Set;
+import java.util.stream.Collectors;
+
+/**
+ * Simple test class for Lookup.
Review Comment:
If this is a test class the name or the package should reflect it.
To me this looks like a utility class.
##########
core/src/main/java/org/apache/calcite/schema/lookup/CompatibilityLookup.java:
##########
@@ -0,0 +1,54 @@
+/*
+ * 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.calcite.schema.lookup;
+
+import org.apache.calcite.linq4j.function.Predicate1;
+
+import org.checkerframework.checker.nullness.qual.Nullable;
+
+import java.util.Set;
+import java.util.function.Function;
+import java.util.function.Supplier;
+import java.util.stream.Collectors;
+
+/**
+ * This class can be used to wrap existing schemas with a pair of {@code
get...}
+ * and {@code get...Names} into a Lookup object.
+ *
+ * @param <T> Element type
+ */
+public class CompatibilityLookup<T> extends IgnoreCaseLookup<T> {
Review Comment:
Frankly, this class does not seem to have anything to do with schemas.
So I would first document what the class does, and then say that it can be
used to wrap schemas.
##########
core/src/main/java/org/apache/calcite/schema/lookup/Named.java:
##########
@@ -0,0 +1,67 @@
+/*
+ * 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.calcite.schema.lookup;
+
+import org.checkerframework.checker.nullness.qual.NonNull;
+import org.checkerframework.checker.nullness.qual.Nullable;
+
+import static java.util.Objects.requireNonNull;
+
+/**
+ * This class is used to hold an object including its name.
Review Comment:
I deduce that names are strings - case sensitive.
I would have done it the other way, having an INamed interface which can be
implemented by various objects. But perhaps this is less disruptive.
##########
core/src/main/java/org/apache/calcite/schema/lookup/Lookup.java:
##########
@@ -0,0 +1,96 @@
+/*
+ * 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.calcite.schema.lookup;
+
+import org.apache.calcite.util.NameMap;
+
+import org.checkerframework.checker.nullness.qual.Nullable;
+
+import java.util.Set;
+import java.util.function.BiFunction;
+
+/**
+ * A casesensitive/insensitive lookup for tables, schems, functions ...
Review Comment:
typo.
Probably you want to add types to the list too.
##########
core/src/test/java/org/apache/calcite/schema/lookup/MapLookup.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.calcite.schema.lookup;
+
+import org.checkerframework.checker.nullness.qual.Nullable;
+
+import java.util.HashMap;
+import java.util.Locale;
+import java.util.Map;
+import java.util.Set;
+import java.util.stream.Collectors;
+
+/**
+ * Simple test class for Lookup.
+ */
+class MapLookup implements Lookup<String> {
+ private final Map<String, String> map;
+ private final Map<String, Named<String>> ignoreCaseMap;
+
+ MapLookup(String... keyAndValues) {
+ this.map = new HashMap<>();
+ for (int i = 0; i < keyAndValues.length - 1; i += 2) {
+ map.put(keyAndValues[i], keyAndValues[i + 1]);
+ }
+ this.ignoreCaseMap = this.map.entrySet().stream()
+ .collect(
+ Collectors.toMap(
+ entry -> entry.getKey().toLowerCase(Locale.ROOT),
+ entry -> new Named<>(entry.getKey(), entry.getValue())));
+ }
+
+ @Override public @Nullable String get(final String name) {
+ return map.get(name);
+ }
+
+ @Override public @Nullable Named<String> getIgnoreCase(final String name) {
+ return ignoreCaseMap.get(name.toLowerCase(Locale.ROOT));
+ }
+
+ @Override public Set<String> getNames(final LikePattern pattern) {
Review Comment:
why is the pattern ignored?
##########
core/src/main/java/org/apache/calcite/schema/lookup/ConcatLookup.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.calcite.schema.lookup;
+
+import org.checkerframework.checker.nullness.qual.Nullable;
+
+import java.util.Set;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+
+/**
+ * This class can be used to concat a list of lookups.
Review Comment:
what does this mean?
please describe the semantics - especially for the case of common names.
##########
core/src/main/java/org/apache/calcite/schema/lookup/IgnoreCaseLookup.java:
##########
@@ -0,0 +1,77 @@
+/*
+ * 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.calcite.schema.lookup;
+
+import org.apache.calcite.util.LazyReference;
+import org.apache.calcite.util.NameMap;
+
+import org.checkerframework.checker.nullness.qual.Nullable;
+
+import java.util.Map;
+import java.util.Set;
+
+/**
+ * An abstract base class for lookups. implementing case insensitive lookup
+ *
+ * @param <T> Element type
+ */
+public abstract class IgnoreCaseLookup<T> implements Lookup<T> {
+
+ private LazyReference<NameMap<String>> nameMap = new LazyReference<>();
Review Comment:
I think this should be documented. If I am guessing right, it maps a name to
it's "canonical" form. If that's true, call it canonicalNameMap.
##########
core/src/main/java/org/apache/calcite/schema/lookup/Lookup.java:
##########
@@ -0,0 +1,96 @@
+/*
+ * 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.calcite.schema.lookup;
+
+import org.apache.calcite.util.NameMap;
+
+import org.checkerframework.checker.nullness.qual.Nullable;
+
+import java.util.Set;
+import java.util.function.BiFunction;
+
+/**
+ * A casesensitive/insensitive lookup for tables, schems, functions ...
+ *
+ * @param <T> Element type
+ */
+public interface Lookup<T> {
+ /**
+ * Returns a named entity with a given name, or null if not found.
+ *
+ * @param name Name
+ * @return Entity, or null
+ */
+ @Nullable T get(String name);
+
+ /**
+ * Returns a named entity with a given name ignoring the case, or null if
not found.
+ *
+ * @param name Name
+ * @return Entity, or null
+ */
+ @Nullable Named<T> getIgnoreCase(String name);
+
+ /**
+ * Returns the names of the entities in matching pattern.
Review Comment:
I assume the pattern can specify case-insensitive matching too (ILIKE).
please specify
##########
core/src/main/java/org/apache/calcite/schema/Schema.java:
##########
@@ -56,9 +59,30 @@
* {@link Schema#getSubSchema(String)}.
*/
public interface Schema {
+
+ /**
+ * Returns a lookup object to find tables.
+ *
+ * @return Lookup
+ */
+ default Lookup<Table> tables() {
+ return new CompatibilityLookup<>(this::getTable, this::getTableNames);
+ }
+
+ /**
+ * Returns a lookup object to find sub schemas.
+ *
+ * @return Lookup
+ */
+ default Lookup<? extends Schema> subSchemas() {
+ return new CompatibilityLookup<>(this::getSubSchema,
this::getSubSchemaNames);
+ }
+
/**
* Returns a table with a given name, or null if not found.
*
+ * <p>Please use {@link Schema#tables()} and {@link Lookup#get(String)}
instead.
Review Comment:
people will not find this comment.
maybe the API should be marked as deprecated.
on the other hand, this API makes sense for some cases - and it should
perhaps have a default implementation doing exactly what you suggest.
There must be some downside to doing that - what is it?
##########
core/src/main/java/org/apache/calcite/schema/lookup/CachedLookup.java:
##########
@@ -0,0 +1,78 @@
+/*
+ * 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.calcite.schema.lookup;
+
+import org.apache.calcite.util.LazyReference;
+import org.apache.calcite.util.NameMap;
+
+import org.checkerframework.checker.nullness.qual.Nullable;
+
+import java.util.Set;
+
+/**
+ * This class can be used to make a snapshot of a lookups.
+ *
+ * @param <T> Element Type
+ */
+public class CachedLookup<T> implements Lookup<T> {
+
+ private final Lookup<T> delegate;
+ private LazyReference<Lookup<T>> cachedDelegate = new LazyReference<>();
Review Comment:
I am assuming this is the "snapshot".
I would call it "snapshot" or "delegateSnapshot".
Is this class mixing two functionalities? Caching and snapshotting look to
me to be separate behaviors.
##########
core/src/main/java/org/apache/calcite/schema/lookup/IgnoreCaseLookup.java:
##########
@@ -0,0 +1,77 @@
+/*
+ * 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.calcite.schema.lookup;
+
+import org.apache.calcite.util.LazyReference;
+import org.apache.calcite.util.NameMap;
+
+import org.checkerframework.checker.nullness.qual.Nullable;
+
+import java.util.Map;
+import java.util.Set;
+
+/**
+ * An abstract base class for lookups. implementing case insensitive lookup
Review Comment:
dot in the middle of the sentence
is this notion of "case sensitivity" dependent on the Locale?
##########
core/src/main/java/org/apache/calcite/schema/lookup/LikePattern.java:
##########
@@ -0,0 +1,92 @@
+/*
+ * 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.calcite.schema.lookup;
+
+import org.apache.calcite.linq4j.function.Predicate1;
+
+import java.util.regex.Pattern;
+
+/**
+ * This class is used as parameter to Lookup.getNames
+ */
+public class LikePattern {
+ private static final String ANY = "%";
+ public final String pattern;
+
+ public LikePattern(String pattern) {
Review Comment:
please document the structure of the pattern.
##########
core/src/main/java/org/apache/calcite/schema/lookup/EmptyLookup.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.calcite.schema.lookup;
+
+import com.google.common.collect.ImmutableSet;
+
+import org.checkerframework.checker.nullness.qual.Nullable;
+
+import java.util.Set;
+
+/**
+ * This class implements an empty Lookup.
Review Comment:
i.e., that contains no named object.
Lookup returns null for all names.
##########
core/src/main/java/org/apache/calcite/jdbc/SimpleCalciteSchema.java:
##########
@@ -100,36 +100,8 @@ private SimpleCalciteSchema(@Nullable CalciteSchema parent,
return null;
}
- @Override protected @Nullable CalciteSchema getImplicitSubSchema(String
schemaName,
- boolean caseSensitive) {
- // Check implicit schemas.
- final String schemaName2 =
- caseSensitive ? schemaName
- : caseInsensitiveLookup(schema.getSubSchemaNames(), schemaName);
- if (schemaName2 == null) {
- return null;
- }
- final Schema s = schema.getSubSchema(schemaName2);
- if (s == null) {
- return null;
- }
- return new SimpleCalciteSchema(this, s, schemaName2);
- }
-
- @Override protected @Nullable TableEntry getImplicitTable(String tableName,
Review Comment:
what happened to all these functions?
(I don't know what they were meant for.)
##########
core/src/main/java/org/apache/calcite/schema/lookup/CompatibilityLookup.java:
##########
@@ -0,0 +1,54 @@
+/*
+ * 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.calcite.schema.lookup;
+
+import org.apache.calcite.linq4j.function.Predicate1;
+
+import org.checkerframework.checker.nullness.qual.Nullable;
+
+import java.util.Set;
+import java.util.function.Function;
+import java.util.function.Supplier;
+import java.util.stream.Collectors;
+
+/**
+ * This class can be used to wrap existing schemas with a pair of {@code
get...}
+ * and {@code get...Names} into a Lookup object.
+ *
+ * @param <T> Element type
+ */
+public class CompatibilityLookup<T> extends IgnoreCaseLookup<T> {
+
+ private final Function<String, @Nullable T> get;
+ private final Supplier<Set<String>> getNames;
+
+ public CompatibilityLookup(Function<String, @Nullable T> get,
Supplier<Set<String>> getNames) {
Review Comment:
Please document the public constructor.
##########
core/src/main/java/org/apache/calcite/jdbc/CachingCalciteSchema.java:
##########
@@ -117,15 +100,14 @@ private CachingCalciteSchema(@Nullable CalciteSchema
parent, Schema schema,
return this.cache;
}
- @Override protected @Nullable CalciteSchema getImplicitSubSchema(String
schemaName,
- boolean caseSensitive) {
- final long now = System.currentTimeMillis();
- final SubSchemaCache subSchemaCache = implicitSubSchemaCache.get(now);
- for (String schemaName2
- : subSchemaCache.names.range(schemaName, caseSensitive)) {
- return subSchemaCache.cache.getUnchecked(schemaName2);
- }
- return null;
+ @Override protected CalciteSchema createSubSchema(Schema schema, String
name) {
+ return new CachingCalciteSchema(this, schema, name);
+ }
+
+ @Override protected <S> Lookup<S> decorateLookup(Lookup<S> lookup) {
Review Comment:
I cannot guess what "decorate" means here
##########
core/src/main/java/org/apache/calcite/schema/lookup/LikePattern.java:
##########
@@ -0,0 +1,92 @@
+/*
+ * 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.calcite.schema.lookup;
+
+import org.apache.calcite.linq4j.function.Predicate1;
+
+import java.util.regex.Pattern;
+
+/**
+ * This class is used as parameter to Lookup.getNames
+ */
+public class LikePattern {
+ private static final String ANY = "%";
+ public final String pattern;
+
+ public LikePattern(String pattern) {
+ if (pattern == null) {
+ pattern = ANY;
+ }
+ this.pattern = pattern;
+ }
+
+ @Override public String toString() {
+ return "LikePattern[" + this.pattern + "]";
+ }
+
+ public Predicate1<String> matcher() {
+ return matcher(pattern);
+ }
+
+ public static LikePattern any() {
+ return new LikePattern(ANY);
+ }
+
+ public static Predicate1<String> matcher(String likePattern) {
+ if (likePattern == null || likePattern.equals(ANY)) {
+ return v1 -> true;
+ }
+ final Pattern regex = likeToRegex(likePattern);
+ return v1 -> regex.matcher(v1).matches();
+ }
+
+ /**
+ * Converts a LIKE-style pattern (where '%' represents a wild-card, escaped
+ * using '\') to a Java regex.
+ */
+ public static Pattern likeToRegex(String pattern) {
Review Comment:
So this is always case-sensitive. Please document.
##########
core/src/main/java/org/apache/calcite/schema/lookup/IgnoreCaseLookup.java:
##########
@@ -0,0 +1,77 @@
+/*
+ * 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.calcite.schema.lookup;
+
+import org.apache.calcite.util.LazyReference;
+import org.apache.calcite.util.NameMap;
+
+import org.checkerframework.checker.nullness.qual.Nullable;
+
+import java.util.Map;
+import java.util.Set;
+
+/**
+ * An abstract base class for lookups. implementing case insensitive lookup
+ *
+ * @param <T> Element type
+ */
+public abstract class IgnoreCaseLookup<T> implements Lookup<T> {
+
+ private LazyReference<NameMap<String>> nameMap = new LazyReference<>();
+
+ /**
+ * Returns a named entity with a given name, or null if not found.
+ *
+ * @param name Name
+ * @return Entity, or null
+ */
+ @Override public abstract @Nullable T get(String name);
+
+ /**
+ * Returns a named entity with a given name ignoring the case, or null if
not found.
+ *
+ * @param name Name
+ * @return Entity, or null
+ */
+ @Override @Nullable public Named<T> getIgnoreCase(String name) {
+ int retryCounter = 0;
+ while (true) {
+ Map.Entry<String, String> entry = nameMap.getOrCompute(this::loadNames)
+ .range(name, false)
+ .firstEntry();
+ if (entry != null) {
+ T result = get(entry.getValue());
+ return result == null ? null : new Named<>(entry.getKey(), result);
+ }
+ retryCounter++;
+ if (retryCounter > 1) {
+ return null;
+ }
+ nameMap.reset();
Review Comment:
I don't get the point of this, please explain
##########
core/src/main/java/org/apache/calcite/adapter/jdbc/JdbcSchema.java:
##########
@@ -237,80 +252,86 @@ public DataSource getDataSource() {
return Schemas.subSchemaExpression(parentSchema, name, JdbcSchema.class);
}
- protected Multimap<String, Function> getFunctions() {
- // TODO: populate map from JDBC metadata
- return ImmutableMultimap.of();
+ private Stream<MetaImpl.MetaTable> getMetaTableStream(String
tableNamePattern) {
+ final Pair<@Nullable String, @Nullable String> catalogSchema =
getCatalogSchema();
+ final Stream<MetaImpl.MetaTable> tableDefs;
+ Connection connection = null;
+ ResultSet resultSet = null;
+ try {
+ connection = dataSource.getConnection();
+ final DatabaseMetaData metaData = connection.getMetaData();
+ resultSet =
+ metaData.getTables(catalogSchema.left, catalogSchema.right,
tableNamePattern, null);
+ tableDefs = asStream(connection, resultSet)
+ .map(JdbcSchema::metaDataMapper);
+ } catch (SQLException e) {
+ close(connection, null, resultSet);
+ throw new RuntimeException(
+ "Exception while reading tables", e);
+ }
+ return tableDefs;
}
- @Override public final Collection<Function> getFunctions(String name) {
- return getFunctions().get(name); // never null
+ private static Stream<ResultSet> asStream(Connection connection, ResultSet
resultSet) {
+ return StreamSupport.stream(
+ new Spliterators.AbstractSpliterator<ResultSet>(
+ Long.MAX_VALUE, Spliterator.ORDERED) {
+ @Override public boolean tryAdvance(Consumer<? super ResultSet>
action) {
+ try {
+ if (!resultSet.next()) {
+ return false;
+ }
+ action.accept(resultSet);
+ return true;
+ } catch (SQLException ex) {
+ throw new RuntimeException(ex);
+ }
+ }
+ }, false).onClose(() -> close(connection, null, resultSet));
}
- @Override public final Set<String> getFunctionNames() {
- return getFunctions().keySet();
+ private JdbcTable jdbcTableMapper(MetaImpl.MetaTable tableDef) {
+ return new JdbcTable(this, tableDef.tableCat, tableDef.tableSchem,
tableDef.tableName,
+ getTableType(tableDef.tableType));
}
- private ImmutableMap<String, JdbcTable> computeTables() {
- Connection connection = null;
- ResultSet resultSet = null;
+ private static MetaImpl.MetaTable metaDataMapper(ResultSet resultSet) {
try {
- connection = dataSource.getConnection();
- final Pair<@Nullable String, @Nullable String> catalogSchema =
getCatalogSchema(connection);
- final String catalog = catalogSchema.left;
- final String schema = catalogSchema.right;
- final Iterable<MetaImpl.MetaTable> tableDefs;
- Foo threadMetadata = THREAD_METADATA.get();
- if (threadMetadata != null) {
- tableDefs = threadMetadata.apply(catalog, schema);
- } else {
- final List<MetaImpl.MetaTable> tableDefList = new ArrayList<>();
- final DatabaseMetaData metaData = connection.getMetaData();
- resultSet = metaData.getTables(catalog, schema, null, null);
- while (resultSet.next()) {
- final String catalogName = resultSet.getString(1);
- final String schemaName = resultSet.getString(2);
- final String tableName = resultSet.getString(3);
- final String tableTypeName = resultSet.getString(4);
- tableDefList.add(
- new MetaImpl.MetaTable(catalogName, schemaName, tableName,
- tableTypeName));
- }
- tableDefs = tableDefList;
- }
+ return new MetaImpl.MetaTable(intern(resultSet.getString(1)),
intern(resultSet.getString(2)),
+ intern(resultSet.getString(3)),
+ intern(resultSet.getString(4)));
+ } catch (SQLException e) {
+ throw new RuntimeException(e);
+ }
+ }
- final ImmutableMap.Builder<String, JdbcTable> builder =
- ImmutableMap.builder();
- for (MetaImpl.MetaTable tableDef : tableDefs) {
- // Clean up table type. In particular, this ensures that 'SYSTEM
TABLE',
- // returned by Phoenix among others, maps to TableType.SYSTEM_TABLE.
- // We know enum constants are upper-case without spaces, so we can't
- // make things worse.
- //
- // PostgreSQL returns tableTypeName==null for pg_toast* tables
- // This can happen if you start JdbcSchema off a "public" PG schema
- // The tables are not designed to be queried by users, however we do
- // not filter them as we keep all the other table types.
- final String tableTypeName2 =
- tableDef.tableType == null
+ private static @Nullable String intern(@Nullable String string) {
+ if (string == null) {
+ return null;
+ }
+ return string.intern();
Review Comment:
I believe that the default Java string interning mechanism has a tiny set
reserved, and can get very inefficient for a large number of strings. Is this
controlled anywhere?
##########
core/src/main/java/org/apache/calcite/schema/lookup/LoadingCacheLookup.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.calcite.schema.lookup;
+
+import com.google.common.cache.CacheBuilder;
+import com.google.common.cache.CacheLoader;
+import com.google.common.cache.LoadingCache;
+import com.google.common.util.concurrent.UncheckedExecutionException;
+
+import org.checkerframework.checker.nullness.qual.Nullable;
+
+import java.util.Set;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.TimeUnit;
+
+import static java.util.Objects.requireNonNull;
+
+/**
+ * This class can be used to cache lookups.
Review Comment:
Please say what it does, before you can say what it can be used for.
The important aspect seems to be some kind of cache expiry mechanism.
I wonder about consistency. Can you say something about this?
I am guessing this assumes (and perhaps schemas do too) that objects are
only inserted, and never deleted?
##########
core/src/main/java/org/apache/calcite/adapter/jdbc/JdbcSchema.java:
##########
@@ -237,80 +252,86 @@ public DataSource getDataSource() {
return Schemas.subSchemaExpression(parentSchema, name, JdbcSchema.class);
}
- protected Multimap<String, Function> getFunctions() {
- // TODO: populate map from JDBC metadata
- return ImmutableMultimap.of();
+ private Stream<MetaImpl.MetaTable> getMetaTableStream(String
tableNamePattern) {
+ final Pair<@Nullable String, @Nullable String> catalogSchema =
getCatalogSchema();
+ final Stream<MetaImpl.MetaTable> tableDefs;
+ Connection connection = null;
+ ResultSet resultSet = null;
+ try {
+ connection = dataSource.getConnection();
+ final DatabaseMetaData metaData = connection.getMetaData();
+ resultSet =
+ metaData.getTables(catalogSchema.left, catalogSchema.right,
tableNamePattern, null);
+ tableDefs = asStream(connection, resultSet)
+ .map(JdbcSchema::metaDataMapper);
+ } catch (SQLException e) {
+ close(connection, null, resultSet);
+ throw new RuntimeException(
+ "Exception while reading tables", e);
+ }
+ return tableDefs;
}
- @Override public final Collection<Function> getFunctions(String name) {
- return getFunctions().get(name); // never null
+ private static Stream<ResultSet> asStream(Connection connection, ResultSet
resultSet) {
+ return StreamSupport.stream(
+ new Spliterators.AbstractSpliterator<ResultSet>(
+ Long.MAX_VALUE, Spliterator.ORDERED) {
+ @Override public boolean tryAdvance(Consumer<? super ResultSet>
action) {
+ try {
+ if (!resultSet.next()) {
+ return false;
+ }
+ action.accept(resultSet);
+ return true;
+ } catch (SQLException ex) {
+ throw new RuntimeException(ex);
+ }
+ }
+ }, false).onClose(() -> close(connection, null, resultSet));
}
- @Override public final Set<String> getFunctionNames() {
- return getFunctions().keySet();
+ private JdbcTable jdbcTableMapper(MetaImpl.MetaTable tableDef) {
+ return new JdbcTable(this, tableDef.tableCat, tableDef.tableSchem,
tableDef.tableName,
+ getTableType(tableDef.tableType));
}
- private ImmutableMap<String, JdbcTable> computeTables() {
- Connection connection = null;
- ResultSet resultSet = null;
+ private static MetaImpl.MetaTable metaDataMapper(ResultSet resultSet) {
try {
- connection = dataSource.getConnection();
- final Pair<@Nullable String, @Nullable String> catalogSchema =
getCatalogSchema(connection);
- final String catalog = catalogSchema.left;
- final String schema = catalogSchema.right;
- final Iterable<MetaImpl.MetaTable> tableDefs;
- Foo threadMetadata = THREAD_METADATA.get();
- if (threadMetadata != null) {
- tableDefs = threadMetadata.apply(catalog, schema);
- } else {
- final List<MetaImpl.MetaTable> tableDefList = new ArrayList<>();
- final DatabaseMetaData metaData = connection.getMetaData();
- resultSet = metaData.getTables(catalog, schema, null, null);
- while (resultSet.next()) {
- final String catalogName = resultSet.getString(1);
- final String schemaName = resultSet.getString(2);
- final String tableName = resultSet.getString(3);
- final String tableTypeName = resultSet.getString(4);
- tableDefList.add(
- new MetaImpl.MetaTable(catalogName, schemaName, tableName,
- tableTypeName));
- }
- tableDefs = tableDefList;
- }
+ return new MetaImpl.MetaTable(intern(resultSet.getString(1)),
intern(resultSet.getString(2)),
+ intern(resultSet.getString(3)),
+ intern(resultSet.getString(4)));
+ } catch (SQLException e) {
+ throw new RuntimeException(e);
+ }
+ }
- final ImmutableMap.Builder<String, JdbcTable> builder =
- ImmutableMap.builder();
- for (MetaImpl.MetaTable tableDef : tableDefs) {
- // Clean up table type. In particular, this ensures that 'SYSTEM
TABLE',
- // returned by Phoenix among others, maps to TableType.SYSTEM_TABLE.
- // We know enum constants are upper-case without spaces, so we can't
- // make things worse.
- //
- // PostgreSQL returns tableTypeName==null for pg_toast* tables
- // This can happen if you start JdbcSchema off a "public" PG schema
- // The tables are not designed to be queried by users, however we do
- // not filter them as we keep all the other table types.
- final String tableTypeName2 =
- tableDef.tableType == null
+ private static @Nullable String intern(@Nullable String string) {
+ if (string == null) {
+ return null;
+ }
+ return string.intern();
+ }
+
+ private static TableType getTableType(String tableTypeName) {
+ // Clean up table type. In particular, this ensures that 'SYSTEM TABLE',
+ // returned by Phoenix among others, maps to TableType.SYSTEM_TABLE.
+ // We know enum constants are upper-case without spaces, so we can't
+ // make things worse.
Review Comment:
worse than what?
##########
core/src/main/java/org/apache/calcite/schema/lookup/LikePattern.java:
##########
@@ -0,0 +1,92 @@
+/*
+ * 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.calcite.schema.lookup;
+
+import org.apache.calcite.linq4j.function.Predicate1;
+
+import java.util.regex.Pattern;
+
+/**
+ * This class is used as parameter to Lookup.getNames
Review Comment:
you should say what the class does, not what it is used for.
--
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]
