From 9898a0b0e24e7e0d1fa08874c575befd8f4ddaf8 Mon Sep 17 00:00:00 2001 From: Leon van Zantvoort Date: Tue, 14 Jul 2026 07:54:36 +0200 Subject: [PATCH] perf: skip column decoding for cached entities and precompute constructor metadata Queries that repeat the same entity across rows (joins over eager foreign keys) paid for decoding the parent's columns on every row even though the mapper's early cache lookup skips construction on a hit. The new ColumnSkipper probes the same caches before column extraction: for each entity region it decodes only the columns up to and including the primary key and, on a hit, leaves the remaining columns undecoded. The mapper then resolves the entity from the primary key alone. Column access stays in ascending order, and interner hits are pinned in a strong reference list until the next row so a garbage collection between probe and mapping cannot clear them. Skip regions are computed once per record type alongside the compiled argument plan. Constructing a record also carried per-invocation reflective overhead: getParameterTypes and getParameters clone their arrays on every call, setAccessible ran per call, and the enum step resolved its mapper per value. Constructor metadata (parameter names, non-null flags, primitive flags, the accessible constructor) is now precomputed and cached per constructor, and enum mappers are resolved once at plan compilation. The integration test counts positional column reads through a JDBC proxy and asserts the exact expected number for the pet-owner-city graph, covering nested regions, interner identity, and null foreign keys. --- .../orm/core/template/impl/ColumnSkipper.java | 220 ++++++++++++++++++ .../orm/core/template/impl/ObjectMapper.java | 12 + .../template/impl/ObjectMapperFactory.java | 71 ++++-- .../st/orm/core/template/impl/QueryImpl.java | 11 +- .../orm/core/template/impl/RecordMapper.java | 112 ++++++++- .../core/ColumnSkipperIntegrationTest.java | 161 +++++++++++++ 6 files changed, 560 insertions(+), 27 deletions(-) create mode 100644 storm-core/src/main/java/st/orm/core/template/impl/ColumnSkipper.java create mode 100644 storm-core/src/test/java/st/orm/core/ColumnSkipperIntegrationTest.java diff --git a/storm-core/src/main/java/st/orm/core/template/impl/ColumnSkipper.java b/storm-core/src/main/java/st/orm/core/template/impl/ColumnSkipper.java new file mode 100644 index 000000000..0bd84aca7 --- /dev/null +++ b/storm-core/src/main/java/st/orm/core/template/impl/ColumnSkipper.java @@ -0,0 +1,220 @@ +/* + * Copyright 2024 - 2026 the original author or authors. + * + * Licensed 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 + * + * https://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 st.orm.core.template.impl; + +import jakarta.annotation.Nonnull; +import jakarta.annotation.Nullable; +import java.lang.reflect.Constructor; +import java.sql.SQLException; +import java.util.ArrayList; +import java.util.List; +import st.orm.Entity; +import st.orm.StormConfig; +import st.orm.core.spi.CacheRetention; +import st.orm.core.spi.EntityCache; +import st.orm.core.spi.TransactionContext; +import st.orm.core.spi.WeakInterner; +import st.orm.core.template.SqlTemplateException; + +/** + * Skips JDBC column decoding for entity column regions whose entity is already cached. + * + *

Queries that return duplicate entity references (joins that repeat the same parent entity across rows) pay + * for decoding the parent's columns on every row, even though the mapper's early cache lookup skips construction + * on a hit. This class probes the same caches before column extraction: for each entity region it decodes + * only the columns up to and including the primary key, and on a cache hit leaves the remaining columns undecoded. + * The mapper's early lookup then resolves the entity from the primary key alone and never reads the skipped + * slots.

+ * + *

Columns are always accessed in ascending order, so drivers that only support forward column access within a + * row are unaffected. Entities found in the query-scoped {@link WeakInterner} are pinned in a strong reference + * list until the next row is read, guaranteeing that the mapper's subsequent lookup of the same key succeeds even + * if a garbage collection runs in between (the interner holds entities weakly).

+ * + *

The cache policy mirrors the mapper's early lookup exactly: nested entity regions consult the + * transaction-scoped {@link EntityCache} when the isolation level is {@code REPEATABLE_READ} or higher and the + * query-scoped {@link WeakInterner} otherwise; the top-level region only consults the {@link EntityCache}, since + * top-level records are never interned.

+ * + *

This class is not thread-safe. A new instance is expected to be created for each query execution.

+ */ +final class ColumnSkipper { + + /** + * A flat column region occupied by an entity that is eligible for the early cache lookup. + * + * @param start the absolute offset of the region's first column. + * @param end the absolute offset just past the region's last column. + * @param pkOffset the absolute offset of the first primary key column. + * @param pkColumnCount the number of columns the primary key spans. + * @param pkConstructor the constructor for composite primary keys (null for single-column keys). + * @param entityType the entity type occupying the region. + * @param topLevel whether the region covers the top-level record rather than a nested entity. + */ + record SkipRegion(int start, + int end, + int pkOffset, + int pkColumnCount, + @Nullable Constructor pkConstructor, + @Nonnull Class> entityType, + boolean topLevel) {} + + /** + * Reads and converts a single JDBC column value. + */ + @FunctionalInterface + interface ColumnReader { + + /** + * Reads the column at the given zero-based index. + * + * @param index the zero-based column index. + * @return the converted column value, or {@code null} if the column is SQL NULL. + * @throws SQLException if a database access error occurs. + */ + @Nullable + Object read(int index) throws SQLException; + } + + private final List regions; + private final WeakInterner interner; + @Nullable + private final TransactionContext context; + @Nullable + private final EntityCache, ?> topLevelCache; + private final boolean topLevelCacheReadEnabled; + + /** Strong references to interner hits for the current row; prevents collection before the mapper's lookup. */ + private final List> pinned = new ArrayList<>(); + + /** + * Creates a new column skipper. + * + * @param regions the entity regions eligible for skipping, ordered by start offset; outer regions precede the + * regions they contain. + * @param interner the query-scoped interner, shared with the mapper. + * @param context the transaction context, or null if not in a transaction. + * @param topLevelCache the entity cache for the top-level record, or null if not applicable. + * @param topLevelCacheReadEnabled whether cache reads are enabled for the top-level record. + */ + ColumnSkipper(@Nonnull List regions, + @Nonnull WeakInterner interner, + @Nullable TransactionContext context, + @Nullable EntityCache, ?> topLevelCache, + boolean topLevelCacheReadEnabled) { + this.regions = regions; + this.interner = interner; + this.context = context; + this.topLevelCache = topLevelCache; + this.topLevelCacheReadEnabled = topLevelCacheReadEnabled; + } + + /** + * Reads a single row into {@code args}, skipping the non-key columns of entity regions that hit the cache. + * Skipped slots are left {@code null}; the mapper never reads them, as its early lookup returns the cached + * instance from the primary key alone. + * + * @param args the flat argument array to fill; its length must match the query's column count. + * @param reader reads and converts a single column value. + * @throws SQLException if a database access error occurs. + * @throws SqlTemplateException if a composite primary key cannot be constructed. + */ + void readRow(@Nonnull Object[] args, @Nonnull ColumnReader reader) throws SQLException, SqlTemplateException { + pinned.clear(); + boolean cacheReadEnabled = context != null && context.isRepeatableRead(); + int i = 0; + for (SkipRegion region : regions) { + if (region.start() < i) { + // The region lies within an outer region that was already skipped. + continue; + } + for (; i < region.start(); i++) { + args[i] = reader.read(i); + } + // Decode up to and including the primary key columns; column access stays in ascending order. + int pkEnd = region.pkOffset() + region.pkColumnCount(); + for (; i < pkEnd; i++) { + args[i] = reader.read(i); + } + Object pk = extractPk(args, region); + if (pk != null && isCached(region, pk, cacheReadEnabled)) { + i = region.end(); + } + } + for (; i < args.length; i++) { + args[i] = reader.read(i); + } + } + + /** + * Extracts the primary key for the given region from the decoded columns. + * + * @param args the flat argument array with the region's primary key columns decoded. + * @param region the region to extract the primary key for. + * @return the primary key value, or {@code null} if any key column is null or the key cannot be constructed. + */ + @Nullable + private Object extractPk(@Nonnull Object[] args, @Nonnull SkipRegion region) throws SqlTemplateException { + if (region.pkColumnCount() == 1) { + return args[region.pkOffset()]; + } + if (region.pkConstructor() == null) { + return null; + } + Object[] pkArgs = new Object[region.pkColumnCount()]; + for (int i = 0; i < region.pkColumnCount(); i++) { + Object arg = args[region.pkOffset() + i]; + if (arg == null) { + return null; // Null in composite PK means no valid PK. + } + pkArgs[i] = arg; + } + return ObjectMapperFactory.construct(region.pkConstructor(), pkArgs, region.pkOffset()); + } + + /** + * Checks whether the entity for the given region and primary key is cached, mirroring the cache policy of the + * mapper's early lookup. + * + * @param region the region being probed. + * @param pk the primary key value. + * @param cacheReadEnabled whether transaction-scoped cache reads are enabled for this row. + * @return {@code true} if the entity is cached and decoding the region's remaining columns can be skipped. + */ + private boolean isCached(@Nonnull SkipRegion region, @Nonnull Object pk, boolean cacheReadEnabled) { + if (region.topLevel()) { + // Top-level records are never interned; only the entity cache applies. + if (topLevelCache == null || !topLevelCacheReadEnabled) { + return false; + } + //noinspection unchecked,rawtypes + return ((EntityCache) topLevelCache).get(pk).isPresent(); + } + if (cacheReadEnabled) { + //noinspection unchecked + var entityCache = (EntityCache, ?>) context.entityCache( + region.entityType(), CacheRetention.fromConfig(StormConfig.defaults())); + //noinspection unchecked,rawtypes + return ((EntityCache) entityCache).get(pk).isPresent(); + } + Entity cached = interner.get(region.entityType(), pk); + if (cached != null) { + pinned.add(cached); + return true; + } + return false; + } +} diff --git a/storm-core/src/main/java/st/orm/core/template/impl/ObjectMapper.java b/storm-core/src/main/java/st/orm/core/template/impl/ObjectMapper.java index 709e0458e..0dd580121 100644 --- a/storm-core/src/main/java/st/orm/core/template/impl/ObjectMapper.java +++ b/storm-core/src/main/java/st/orm/core/template/impl/ObjectMapper.java @@ -16,6 +16,7 @@ package st.orm.core.template.impl; import jakarta.annotation.Nonnull; +import jakarta.annotation.Nullable; import st.orm.core.template.SqlTemplateException; /** @@ -33,6 +34,17 @@ public interface ObjectMapper { */ Class[] getParameterTypes() throws SqlTemplateException; + /** + * Returns the column skipper that allows the row reader to skip decoding columns of cached entities, or + * {@code null} if this mapper does not support column skipping. + * + * @return the column skipper, or {@code null} if not applicable. + */ + @Nullable + default ColumnSkipper columnSkipper() { + return null; + } + /** * Creates a new instance of the type. * diff --git a/storm-core/src/main/java/st/orm/core/template/impl/ObjectMapperFactory.java b/storm-core/src/main/java/st/orm/core/template/impl/ObjectMapperFactory.java index 4dc069e7a..f8fec1546 100644 --- a/storm-core/src/main/java/st/orm/core/template/impl/ObjectMapperFactory.java +++ b/storm-core/src/main/java/st/orm/core/template/impl/ObjectMapperFactory.java @@ -160,25 +160,28 @@ private static T construct(@Nonnull Constructor constructor, @Nonnull Obj */ static T construct(@Nonnull Constructor constructor, @Nonnull Object[] args, int offset) throws SqlTemplateException { try { - Class[] parameterTypes = constructor.getParameterTypes(); - Parameter[] parameters = constructor.getParameters(); - for (int i = 0; i < parameterTypes.length; i++) { - Object arg = args[i]; - Class paramType = parameterTypes[i]; - if (arg == null) { - if (isNonnull(parameters[i])) { + // Constructor metadata is precomputed and cached: per-invocation getParameterTypes/getParameters calls + // clone their arrays, which is measurable on the row mapping hot path. + ConstructorMeta meta = CONSTRUCTOR_META.computeIfAbsent(constructor, ObjectMapperFactory::constructorMeta); + boolean[] nonNull = meta.nonNull(); + boolean[] primitive = meta.primitive(); + for (int i = 0; i < nonNull.length; i++) { + if (args[i] == null) { + if (nonNull[i]) { throw new SqlTemplateException("Database returned NULL for non-nullable field '%s.%s' at column position %d. Either %s, ensure the column has a NOT NULL constraint with a default value, or verify the query returns the expected data." - .formatted(constructor.getDeclaringClass().getSimpleName(), parameters[i].getName(), offset + i + 1, nullableHint(constructor.getDeclaringClass()))); + .formatted(constructor.getDeclaringClass().getSimpleName(), meta.parameterNames()[i], offset + i + 1, nullableHint(constructor.getDeclaringClass()))); } - if (paramType.isPrimitive()) { + if (primitive[i]) { throw new SqlTemplateException("Database returned NULL for primitive field '%s.%s' at column position %d. Primitive types cannot hold null values. Change the field type to its wrapper class (e.g., int to Integer) and %s, or ensure the column is NOT NULL." - .formatted(constructor.getDeclaringClass().getSimpleName(), parameters[i].getName(), offset + i + 1, nullableHint(constructor.getDeclaringClass()))); + .formatted(constructor.getDeclaringClass().getSimpleName(), meta.parameterNames()[i], offset + i + 1, nullableHint(constructor.getDeclaringClass()))); } } } - constructor.setAccessible(true); try { - return constructor.newInstance(args); + // Use the map's constructor instance: its accessible flag was set before publication, so the + // happens-before edge of the concurrent map makes the flag visible to all threads. + //noinspection unchecked + return (T) meta.constructor().newInstance(args); } catch (InvocationTargetException e) { throw e.getTargetException(); } @@ -189,6 +192,37 @@ static T construct(@Nonnull Constructor constructor, @Nonnull Object[] ar } } + /** + * Precomputed constructor metadata for the row mapping hot path. + * + * @param constructor the constructor with its accessible flag set. + * @param parameterNames the parameter names, for error messages. + * @param nonNull whether each parameter is marked as non-null. + * @param primitive whether each parameter is a primitive type. + */ + private record ConstructorMeta(@Nonnull Constructor constructor, + @Nonnull String[] parameterNames, + @Nonnull boolean[] nonNull, + @Nonnull boolean[] primitive) {} + + /** Cache of precomputed constructor metadata, keyed by constructor. Thread-safe for concurrent access. */ + private static final Map, ConstructorMeta> CONSTRUCTOR_META = new ConcurrentHashMap<>(); + + private static ConstructorMeta constructorMeta(@Nonnull Constructor constructor) { + Class[] parameterTypes = constructor.getParameterTypes(); + Parameter[] parameters = constructor.getParameters(); + String[] parameterNames = new String[parameters.length]; + boolean[] nonNull = new boolean[parameters.length]; + boolean[] primitive = new boolean[parameters.length]; + for (int i = 0; i < parameters.length; i++) { + parameterNames[i] = parameters[i].getName(); + nonNull[i] = isNonnull(parameters[i]); + primitive[i] = parameterTypes[i].isPrimitive(); + } + constructor.setAccessible(true); + return new ConstructorMeta(constructor, parameterNames, nonNull, primitive); + } + @SuppressWarnings("unchecked") private static final Class KOTLIN_METADATA = ((Supplier>) () -> { try { @@ -234,20 +268,17 @@ static String nullableHint(@Nonnull Class clazz) { } }).get(); - private static final Map NONNULL_CACHE = new ConcurrentHashMap<>(); - /** * Returns true if the specified parameter is marked as non-null, false otherwise. * + *

Only called when building {@link ConstructorMeta}, which caches the result per constructor parameter.

+ * * @param parameter the parameter to check for a non-null characteristics. * @return true if the specified parameter is marked as non-null, false otherwise. */ static boolean isNonnull(@Nonnull Parameter parameter) { - // Use the cache to return the result if it's already calculated - return NONNULL_CACHE.computeIfAbsent(parameter, param -> - param.isAnnotationPresent(PK.class) - || (JAVAX_NONNULL != null && param.isAnnotationPresent(JAVAX_NONNULL)) - || (JAKARTA_NONNULL != null && param.isAnnotationPresent(JAKARTA_NONNULL)) - ); + return parameter.isAnnotationPresent(PK.class) + || (JAVAX_NONNULL != null && parameter.isAnnotationPresent(JAVAX_NONNULL)) + || (JAKARTA_NONNULL != null && parameter.isAnnotationPresent(JAKARTA_NONNULL)); } } diff --git a/storm-core/src/main/java/st/orm/core/template/impl/QueryImpl.java b/storm-core/src/main/java/st/orm/core/template/impl/QueryImpl.java index 9bab3950e..9f977ec3d 100644 --- a/storm-core/src/main/java/st/orm/core/template/impl/QueryImpl.java +++ b/storm-core/src/main/java/st/orm/core/template/impl/QueryImpl.java @@ -623,6 +623,7 @@ protected Spliterator rowSpliterator(@Nonnull ResultSet resultSet, int co } catch (SqlTemplateException e) { throw new PersistenceException(e); } + var columnSkipper = mapper.columnSkipper(); var calendarSupplier = lazy(() -> Calendar.getInstance(TimeZone.getTimeZone(ZoneOffset.UTC))); return new Spliterators.AbstractSpliterator<>(Long.MAX_VALUE, Spliterator.ORDERED) { @Override @@ -632,8 +633,14 @@ public boolean tryAdvance(@Nonnull Consumer action) { return false; } Object[] args = new Object[columnCount]; - for (int i = 0; i < columnCount; i++) { - args[i] = readColumnValue(resultSet, i + 1, types[i], calendarSupplier); + if (columnSkipper != null) { + // Skip decoding non-key columns of entities that are already cached. + columnSkipper.readRow(args, index -> + readColumnValue(resultSet, index + 1, types[index], calendarSupplier)); + } else { + for (int i = 0; i < columnCount; i++) { + args[i] = readColumnValue(resultSet, i + 1, types[i], calendarSupplier); + } } action.accept(mapper.newInstance(args)); return true; diff --git a/storm-core/src/main/java/st/orm/core/template/impl/RecordMapper.java b/storm-core/src/main/java/st/orm/core/template/impl/RecordMapper.java index 0df9c5633..bd2527a05 100644 --- a/storm-core/src/main/java/st/orm/core/template/impl/RecordMapper.java +++ b/storm-core/src/main/java/st/orm/core/template/impl/RecordMapper.java @@ -364,8 +364,12 @@ private static SealedCompiled compileSealedPlan(@Nonnull Class sealedType, * * @param plan the compiled argument plan for adapting flat JDBC args to constructor args. * @param parameterTypes the expanded JDBC column types (flattened from nested records). + * @param skipRegions the column regions of nested entities eligible for skipping decode on cache hits. */ - private record Compiled(@Nonnull ArgumentPlan plan, @Nonnull Class[] parameterTypes, @Nonnull PkInfo pkInfo) {} + private record Compiled(@Nonnull ArgumentPlan plan, + @Nonnull Class[] parameterTypes, + @Nonnull PkInfo pkInfo, + @Nonnull List skipRegions) {} /** Global cache of compiled plans, keyed by record class. Thread-safe for concurrent access. */ private static final ConcurrentMap, Compiled> COMPILED = new ConcurrentHashMap<>(); @@ -386,7 +390,10 @@ private static Compiled compiledFor(@Nonnull RecordType type, PkInfo pkInfo = Entity.class.isAssignableFrom(type.type()) ? calculatePkInfo(type) : PkInfo.NONE; - return new Compiled(compilePlan(type), expandParameterTypes(type, refFactory), pkInfo); + List skipRegions = new ArrayList<>(); + collectSkipRegions(type, 0, skipRegions); + return new Compiled(compilePlan(type), expandParameterTypes(type, refFactory), pkInfo, + List.copyOf(skipRegions)); } catch (SqlTemplateException e) { throw new RuntimeException(e); } @@ -460,12 +467,19 @@ private static ObjectMapper wrapConstructor(@Nonnull RecordType type, entityCache = null; } PkInfo pkInfo = compiled.pkInfo(); + ColumnSkipper columnSkipper = createColumnSkipper(type, compiled, cacheReadEnabled, entityCache, + interner, transactionContext); return new ObjectMapper<>() { @Override public Class[] getParameterTypes() { return compiled.parameterTypes(); } + @Override + public ColumnSkipper columnSkipper() { + return columnSkipper; + } + @SuppressWarnings("unchecked") @Override public T newInstance(@Nonnull Object[] args) throws SqlTemplateException { @@ -531,6 +545,48 @@ private Object extractPk(@Nonnull Object[] args, @Nonnull PkInfo pkInfo) throws }; } + /** + * Creates the column skipper for the given compiled plan, or {@code null} when no column region can ever be + * skipped for the mapped type. + * + *

The top-level region mirrors the early cache lookup in the mapper's {@code newInstance}: it only applies + * when cache reads are enabled, as top-level records are never interned.

+ * + * @param type the mapped record type. + * @param compiled the compiled plan for the mapped type. + * @param cacheReadEnabled whether transaction-scoped cache reads are enabled. + * @param entityCache the entity cache for the top-level record, or null if not applicable. + * @param interner the query-scoped interner, shared with the mapper. + * @param transactionContext the transaction context, or null if not in a transaction. + * @return the column skipper, or {@code null} if no region is eligible. + */ + @Nullable + private static ColumnSkipper createColumnSkipper(@Nonnull RecordType type, + @Nonnull Compiled compiled, + boolean cacheReadEnabled, + @Nullable EntityCache, ?> entityCache, + @Nonnull WeakInterner interner, + @Nullable TransactionContext transactionContext) { + PkInfo pkInfo = compiled.pkInfo(); + boolean topLevel = entityCache != null && cacheReadEnabled && pkInfo.offset() >= 0 + && (pkInfo.columnCount() == 1 || pkInfo.constructor() != null); + List skipRegions = compiled.skipRegions(); + if (!topLevel && skipRegions.isEmpty()) { + return null; + } + List regions; + if (topLevel) { + regions = new ArrayList<>(skipRegions.size() + 1); + //noinspection unchecked + regions.add(new ColumnSkipper.SkipRegion(0, compiled.parameterTypes().length, pkInfo.offset(), + pkInfo.columnCount(), pkInfo.constructor(), (Class>) type.type(), true)); + regions.addAll(skipRegions); + } else { + regions = skipRegions; + } + return new ColumnSkipper(regions, interner, transactionContext, entityCache, cacheReadEnabled); + } + /** * Expands the specified parameter types to include the types of record components. * @@ -765,16 +821,17 @@ interface ConverterInvoker { * */ private static final class EnumStep implements Step { - private final Class enumType; private final EnumType mapping; private final String ownerSimpleName; private final String fieldName; + private final ObjectMapper enumMapper; private EnumStep(Class enumType, EnumType mapping, String ownerSimpleName, String fieldName) { - this.enumType = enumType; this.mapping = mapping; this.ownerSimpleName = ownerSimpleName; this.fieldName = fieldName; + // Resolved once at plan compilation; the factory lookup is too costly to repeat per row. + this.enumMapper = EnumMapper.getFactory(1, enumType).orElseThrow(); } @Override @@ -800,7 +857,7 @@ public Object apply(@Nonnull Object[] flatArgs, ); } }; - return EnumMapper.getFactory(1, enumType).orElseThrow().newInstance(new Object[]{v}); + return enumMapper.newInstance(new Object[]{v}); } } @@ -1167,6 +1224,51 @@ private static PkInfo calculatePkInfo(@Nonnull RecordType type) throws SqlTempla return new PkInfo(offset, pkColumnCount, pkConstructor); } + /** + * Collects the flat column regions of nested entities that are eligible for the early cache lookup, in + * ascending column order with outer regions preceding the regions they contain. + * + *

These regions allow the row reader to skip decoding the non-key columns of an entity that is already + * cached; see {@link ColumnSkipper}. A region is only eligible when the primary key can be extracted directly + * from the flat columns, mirroring the conditions of the early cache lookup in {@link RecordStep}.

+ * + * @param type the record type to analyze. + * @param base the absolute column offset at which the record type starts. + * @param out the list to add the regions to. + */ + private static void collectSkipRegions(@Nonnull RecordType type, + int base, + @Nonnull List out) throws SqlTemplateException { + int cursor = base; + for (RecordField field : type.fields()) { + var converter = getORMConverter(field); + if (converter.isPresent()) { + cursor += converter.get().getParameterCount(); + continue; + } + if (isRecord(field.type())) { + RecordType sub = getRecordType(field.type()); + int totalColumnCount = getParameterCount(sub); + if (Entity.class.isAssignableFrom(sub.type())) { + PkInfo pkInfo = calculatePkInfo(sub); + if (pkInfo.offset() >= 0 && (pkInfo.columnCount() == 1 || pkInfo.constructor() != null)) { + //noinspection unchecked + out.add(new ColumnSkipper.SkipRegion(cursor, cursor + totalColumnCount, + cursor + pkInfo.offset(), pkInfo.columnCount(), pkInfo.constructor(), + (Class>) sub.type(), false)); + } + } + collectSkipRegions(sub, cursor, out); + cursor += totalColumnCount; + } else if (Ref.class.isAssignableFrom(field.type()) && isPolymorphicData(getRefDataType(field))) { + // Polymorphic FK: discriminator + PK columns. + cursor += 2; + } else { + cursor += 1; + } + } + } + /** * Returns the number of JDBC columns a field consumes in the flat args array. * diff --git a/storm-core/src/test/java/st/orm/core/ColumnSkipperIntegrationTest.java b/storm-core/src/test/java/st/orm/core/ColumnSkipperIntegrationTest.java new file mode 100644 index 000000000..5db7ae09d --- /dev/null +++ b/storm-core/src/test/java/st/orm/core/ColumnSkipperIntegrationTest.java @@ -0,0 +1,161 @@ +/* + * Copyright 2024 - 2026 the original author or authors. + * + * Licensed 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 + * + * https://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 st.orm.core; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertSame; + +import java.lang.reflect.InvocationHandler; +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Proxy; +import java.sql.Connection; +import java.sql.PreparedStatement; +import java.sql.ResultSet; +import java.sql.Statement; +import java.util.HashMap; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.concurrent.atomic.AtomicInteger; +import javax.sql.DataSource; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.autoconfigure.orm.jpa.DataJpaTest; +import org.springframework.test.context.ContextConfiguration; +import org.springframework.test.context.junit.jupiter.SpringExtension; +import st.orm.core.model.City; +import st.orm.core.model.Owner; +import st.orm.core.model.Pet; +import st.orm.core.template.ORMTemplate; + +/** + * Verifies that the row reader skips decoding the non-key columns of entities that are already interned within the + * query. The pet graph joins each pet to its owner and the owner's city, so owners and cities repeat across rows; + * only their first occurrence should decode the full column region, subsequent rows should decode the primary key + * alone. + */ +@ExtendWith(SpringExtension.class) +@ContextConfiguration(classes = IntegrationConfig.class) +@DataJpaTest(showSql = false) +public class ColumnSkipperIntegrationTest { + + // Flat column layout of the Pet graph: pet (id, name, birth_date, type ref), then the owner region + // (id, first_name, last_name, address, city region (id, name), telephone, version). + private static final int PET_COLUMNS = 4; + private static final int OWNER_PREFIX_COLUMNS = 3; // Owner columns between the owner PK and the city region. + private static final int CITY_COLUMNS = 2; + private static final int OWNER_SUFFIX_COLUMNS = 2; // Owner columns after the city region. + + @Autowired + private DataSource dataSource; + + @Test + public void testSkipsDecodingColumnsOfInternedEntities() throws Exception { + var counter = new ColumnReadCounter(); + List pets; + try (var connection = dataSource.getConnection()) { + pets = ORMTemplate.of(counter.wrap(connection)).entity(Pet.class).select().getResultList(); + } + + // Repeated owners and cities must resolve to the same instance (query-scoped interning). + Map ownersById = new HashMap<>(); + Map citiesById = new HashMap<>(); + for (Pet pet : pets) { + Owner owner = pet.owner(); + if (owner != null) { + assertSame(ownersById.merge(owner.id(), owner, (a, b) -> a), owner); + City city = owner.address().city(); + if (city != null) { + assertSame(citiesById.merge(city.id(), city, (a, b) -> a), city); + } + } + } + + // Hydration must be unaffected by the skipped columns. + var reference = ORMTemplate.of(dataSource).entity(Pet.class).select().getResultList(); + assertEquals(Set.copyOf(reference), Set.copyOf(pets)); + + // Each row decodes the pet columns and the owner PK. The remaining owner and city columns are only decoded + // on their first occurrence; when the owner PK is null (left join), the full region decodes as usual. + Set seenOwners = new HashSet<>(); + Set seenCities = new HashSet<>(); + int expected = 0; + for (Pet pet : pets) { + expected += PET_COLUMNS + 1; + Owner owner = pet.owner(); + if (owner == null) { + expected += OWNER_PREFIX_COLUMNS + CITY_COLUMNS + OWNER_SUFFIX_COLUMNS; + continue; + } + if (!seenOwners.add(owner.id())) { + continue; + } + expected += OWNER_PREFIX_COLUMNS + 1 + OWNER_SUFFIX_COLUMNS; + City city = owner.address().city(); + if (city == null || seenCities.add(city.id())) { + expected += CITY_COLUMNS - 1; + } + } + assertEquals(expected, counter.reads().get()); + } + + /** + * Wraps a connection so that every positional column getter on the result sets it produces is counted. + */ + private static final class ColumnReadCounter { + private final AtomicInteger reads = new AtomicInteger(); + + AtomicInteger reads() { + return reads; + } + + Connection wrap(Connection connection) { + return (Connection) proxy(connection, Connection.class); + } + + private Object proxy(Object target, Class type) { + InvocationHandler handler = (proxyInstance, method, args) -> { + if (type == ResultSet.class + && method.getName().startsWith("get") + && args != null && args.length > 0 && args[0] instanceof Integer) { + reads.incrementAndGet(); + } + Object result; + try { + result = method.invoke(target, args); + } catch (InvocationTargetException e) { + throw e.getTargetException(); + } + if (result instanceof ResultSet resultSet) { + return proxy(resultSet, ResultSet.class); + } + if (result instanceof PreparedStatement statement) { + return proxy(statement, PreparedStatement.class); + } + if (result instanceof Statement statement) { + return proxy(statement, Statement.class); + } + if (result instanceof Connection connection) { + return proxy(connection, Connection.class); + } + return result; + }; + return Proxy.newProxyInstance(getClass().getClassLoader(), new Class[] { type }, handler); + } + } +}