[#15998] [#15996] RenderTable fixes

This includes: - [#16121] Add Context.scopeParts(Class<Q>): Iterable<Q> to allow for iterating over the values that are currently in scope - [#15998] RenderTable.WHEN_AMBIGUOUS_COLUMNS doesn't cover all ambiguous column name cases - [#15996] RenderTable.WHEN_MULTIPLE_TABLES doesn't work correctly in correlated subqueries
jOOQ · Jan 22, 2024 · ed915ed · ed915ed
1 parent 09494ea
commit ed915ed
Show file tree

Hide file tree

Showing 5 changed files with 109 additions and 16 deletions.
diff --git a/jOOQ/src/main/java/org/jooq/Context.java b/jOOQ/src/main/java/org/jooq/Context.java
@@ -404,6 +404,12 @@ public interface Context<C extends Context<C>> extends ExecuteScope {
     @NotNull
     C scopeRegister(QueryPart part, boolean forceNew, QueryPart mapped);
 
+    /**
+     * Get all values of a type that are in the current scope or higher.
+     */
+    @NotNull
+    <Q extends QueryPart> Iterable<Q> scopeParts(Class<? extends Q> type);
+
     /**
      * Retrieve the registered mapping for a query part in the current scope.
      * <p>

diff --git a/jOOQ/src/main/java/org/jooq/impl/AbstractContext.java b/jOOQ/src/main/java/org/jooq/impl/AbstractContext.java
@@ -831,6 +831,11 @@ public final C scopeRegister(QueryPart part, boolean forceNew) {
         return (C) this;
     }
 
+    @Override
+    public final <Q extends QueryPart> Iterable<Q> scopeParts(Class<? extends Q> type) {
+        return (Iterable<Q>) scopeStack.keyIterable(k -> type.isInstance(k));
+    }
+
     @Override
     public /* non-final */ QueryPart scopeMapping(QueryPart part) {
         return part;

diff --git a/jOOQ/src/main/java/org/jooq/impl/ScopeStack.java b/jOOQ/src/main/java/org/jooq/impl/ScopeStack.java
@@ -43,6 +43,8 @@
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.Map.Entry;
+import java.util.function.BiFunction;
 import java.util.function.Function;
 import java.util.function.IntFunction;
 import java.util.function.Predicate;
@@ -102,7 +104,7 @@ final boolean isEmpty() {
     }
 
     final Iterable<Value<V>> valueIterable() {
-        return () -> new ScopeStackIterator<Value<V>>(Value::lastOf, e -> true);
+        return () -> new ScopeStackIterator<Value<V>>(e -> Value.lastOf(e.getValue()), e -> true);
     }
 
     @Override
@@ -111,11 +113,19 @@ public final Iterator<V> iterator() {
     }
 
     final Iterable<V> iterableAtScopeLevel() {
-        return () -> new ScopeStackIterator<>(list -> list.size() == scopeLevel + 1 ? list.get(scopeLevel) : null, e -> true);
+        return () -> new ScopeStackIterator<>((k, v) -> v.size() == scopeLevel + 1 ? v.get(scopeLevel) : null, e -> true);
     }
 
     final Iterable<V> iterable(Predicate<? super V> filter) {
-        return () -> new ScopeStackIterator<>(list -> list.get(list.size() - 1), filter);
+        return () -> new ScopeStackIterator<>((k, v) -> v.get(v.size() - 1), filter);
+    }
+
+    final Iterable<K> keyIterableAtScopeLevel() {
+        return () -> new ScopeStackIterator<>((k, v) -> v.size() == scopeLevel + 1 ? k : null, e -> true);
+    }
+
+    final Iterable<K> keyIterable(Predicate<? super K> filter) {
+        return () -> new ScopeStackIterator<>((k, v) -> k, filter);
     }
 
     static final record Value<V>(int scopeLevel, V value) {
@@ -131,12 +141,16 @@ static <V> Value<V> lastOf(List<V> list) {
     }
 
     private final class ScopeStackIterator<U> implements Iterator<U> {
-        final Iterator<List<V>>    it = stack().values().iterator();
-        final Function<List<V>, U> valueExtractor;
-        final Predicate<? super U> filter;
-        U                          next;
+        final Iterator<Entry<K, List<V>>>    it = stack().entrySet().iterator();
+        final Function<Entry<K, List<V>>, U> valueExtractor;
+        final Predicate<? super U>           filter;
+        U                                    next;
+
+        ScopeStackIterator(BiFunction<K, List<V>, U> valueExtractor, Predicate<? super U> filter) {
+            this(e -> valueExtractor.apply(e.getKey(), e.getValue()), filter);
+        }
 
-        ScopeStackIterator(Function<List<V>, U> valueExtractor, Predicate<? super U> filter) {
+        ScopeStackIterator(Function<Entry<K, List<V>>, U> valueExtractor, Predicate<? super U> filter) {
             this.valueExtractor = valueExtractor;
             this.filter = filter;
         }
@@ -160,8 +174,8 @@ public U next() {
 
         private U move() {
             for (
-                List<V> list;
-                it.hasNext() && ((list = it.next()).isEmpty() || ((next = valueExtractor.apply(list)) == null) || !filter.test(next));
+                Entry<K, List<V>> e;
+                it.hasNext() && ((e = it.next()).getValue().isEmpty() || ((next = valueExtractor.apply(e)) == null) || !filter.test(next));
                 next = null
             );
 

diff --git a/jOOQ/src/main/java/org/jooq/impl/SelectQueryImpl.java b/jOOQ/src/main/java/org/jooq/impl/SelectQueryImpl.java
@@ -200,12 +200,15 @@
 import static org.jooq.impl.Tools.anyMatch;
 import static org.jooq.impl.Tools.autoAlias;
 import static org.jooq.impl.Tools.camelCase;
+import static org.jooq.impl.Tools.concat;
 import static org.jooq.impl.Tools.containsUnaliasedTable;
 import static org.jooq.impl.Tools.fieldArray;
 import static org.jooq.impl.Tools.hasAmbiguousNames;
+import static org.jooq.impl.Tools.hasAmbiguousNamesInTables;
 import static org.jooq.impl.Tools.isEmpty;
 import static org.jooq.impl.Tools.isNotEmpty;
 import static org.jooq.impl.Tools.isWindow;
+import static org.jooq.impl.Tools.joinedTables;
 import static org.jooq.impl.Tools.map;
 import static org.jooq.impl.Tools.qualify;
 import static org.jooq.impl.Tools.recordType;
@@ -249,6 +252,7 @@
 import java.util.Deque;
 import java.util.HashMap;
 import java.util.HashSet;
+import java.util.Iterator;
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
@@ -1667,6 +1671,7 @@ else if (withReadOnly && NO_SUPPORT_WITH_READ_ONLY.contains(ctx.dialect()))
 
 
 
+    @SuppressWarnings({ "unchecked", "rawtypes" })
     final void accept0(Context<?> context) {
 
 
@@ -1694,16 +1699,25 @@ final void accept0(Context<?> context) {
             }
 
             case WHEN_MULTIPLE_TABLES: {
-                int s = getFrom().size();
-
-                if (knownTableSource() && (s == 0 || s == 1 && !(getFrom().get(0) instanceof JoinTable)))
-                    context.data(DATA_RENDER_TABLE, false);
+                if (knownTableSource()) {
+                    Iterator<Table<?>> it = concat(
+                        joinedTables(getFrom()),
+                        context.scopeParts((Class<Table<?>>) (Class) Table.class)
+                    ).iterator();
+
+                    // [#15996] At least 2 tables are in scope
+                    if (!it.hasNext() || it.next() != null && !it.hasNext())
+                        context.data(DATA_RENDER_TABLE, false);
+                }
 
                 break;
             }
 
             case WHEN_AMBIGUOUS_COLUMNS: {
-                if (knownTableSource() && !hasAmbiguousNames(getSelect()))
+                if (knownTableSource() && !hasAmbiguousNamesInTables(concat(
+                    joinedTables(getFrom()),
+                    context.scopeParts((Class<Table<?>>) (Class) Table.class)
+                )))
                     context.data(DATA_RENDER_TABLE, false);
 
                 break;

diff --git a/jOOQ/src/main/java/org/jooq/impl/Tools.java b/jOOQ/src/main/java/org/jooq/impl/Tools.java
@@ -6159,7 +6159,15 @@ static final boolean isDate(Class<?> t) {
         return t == Date.class || t == LocalDate.class;
     }
 
-    static final boolean hasAmbiguousNames(Collection<? extends Field<?>> fields) {
+    static final boolean hasAmbiguousNamesInTables(Iterable<? extends Table<?>> tables) {
+        if (tables == null)
+            return false;
+
+        Set<String> names = new HashSet<>();
+        return anyMatch(tables, t -> anyMatch(t.fields(), f -> !names.add(f.getName())));
+    }
+
+    static final boolean hasAmbiguousNames(Iterable<? extends Field<?>> fields) {
         if (fields == null)
             return false;
 
@@ -6536,6 +6544,40 @@ static final boolean hasEmbeddedFields(Iterable<? extends Field<?>> fields) {
         return anyMatch(fields, f -> f.getDataType().isEmbeddable());
     }
 
+    static final <E> Iterable<E> concat(Iterable<E> i1, Iterable<E> i2) {
+        return () -> concat(i1.iterator(), i2.iterator());
+    }
+
+    static final <E> Iterator<E> concat(Iterator<E> i1, Iterator<E> i2) {
+        return new Iterator<E>() {
+            boolean first = true;
+
+            @Override
+            public boolean hasNext() {
+                if (first)
+                    if (i1.hasNext())
+                        return true;
+                    else
+                        first = false;
+
+                return i2.hasNext();
+            }
+
+            @Override
+            public E next() {
+                return first ? i1.next() : i2.next();
+            }
+
+            @Override
+            public void remove() {
+                if (first)
+                    i1.remove();
+                else
+                    i2.remove();
+            }
+        };
+    }
+
     static final <E> List<E> collect(Iterable<E> iterable) {
         if (iterable instanceof List<E> l)
             return l;
@@ -7234,6 +7276,18 @@ static final boolean containsUnaliasedTable(Iterable<? extends Table<?>> in, Tab
         return traverseJoins(in, false, r -> r, search(search, Tools::unwrap));
     }
 
+    static final List<Table<?>> joinedTables(Iterable<? extends Table<?>> i) {
+        List<Table<?>> result = new ArrayList<>();
+        traverseJoins(i, result::add);
+        return result;
+    }
+
+    static final List<Table<?>> joinedTables(Table<?> t) {
+        List<Table<?>> result = new ArrayList<>();
+        traverseJoins(t, result::add);
+        return result;
+    }
+
     static final void traverseJoins(Iterable<? extends Table<?>> i, Consumer<? super Table<?>> consumer) {
         for (Table<?> t : i)
             traverseJoins(t, consumer);