From c83775027fb7e840a25d79e743d03cd3890efd74 Mon Sep 17 00:00:00 2001 From: John Brestelli Date: Sat, 15 Aug 2026 12:10:42 -0400 Subject: [PATCH 01/10] Add SpanSource interface and record-class registry An unregistered record class now throws rather than silently falling back to a bare apidb.FeatureLocation join that was never validated for it. --- .../spanlogic/SpanCompositionPlugin.java | 62 +++++++++++++++++++ .../wsfplugin/spanlogic/SpanSourceTest.java | 49 +++++++++++++++ 2 files changed, 111 insertions(+) create mode 100644 WSFPlugin/src/test/java/org/apidb/apicomplexa/wsfplugin/spanlogic/SpanSourceTest.java diff --git a/WSFPlugin/src/main/java/org/apidb/apicomplexa/wsfplugin/spanlogic/SpanCompositionPlugin.java b/WSFPlugin/src/main/java/org/apidb/apicomplexa/wsfplugin/spanlogic/SpanCompositionPlugin.java index 51869de5..56448abc 100644 --- a/WSFPlugin/src/main/java/org/apidb/apicomplexa/wsfplugin/spanlogic/SpanCompositionPlugin.java +++ b/WSFPlugin/src/main/java/org/apidb/apicomplexa/wsfplugin/spanlogic/SpanCompositionPlugin.java @@ -101,6 +101,42 @@ private static class Flag { private boolean hasSnp = false; } + /** + * Where a record type's genomic coordinates come from. One implementation per record + * class that may be an input to colocation. + * + * Implementations MUST alias their location table "fl" -- makeRegion() hardcodes that + * prefix when building the region expressions interpolated into every builder. + */ + interface SpanSource { + + /** Full CREATE TABLE statement producing the per-record span temp table. */ + String createTableSql(String tableName, String[] region, String cacheSql); + + /** + * True for a point feature with no meaningful strand. Suppresses the same-strand / + * opposite-strand filter for the whole comparison; without it, "same strand" would + * silently match only forward-strand records. + */ + default boolean isStrandless() { + return false; + } + } + + private static final Map SPAN_SOURCES = Map.of( + "TranscriptRecordClasses.TranscriptRecordClass", new TranscriptSpanSource(), + "DynSpanRecordClasses.DynSpanRecordClass", new DynSpanSource(), + "VariantRecordClasses.VariantRecordClass", new VariantSpanSource()); + + static SpanSource spanSourceFor(String recordClassName) throws WdkModelException { + SpanSource source = SPAN_SOURCES.get(recordClassName); + if (source == null) { + throw new WdkModelException("Genomic colocation is not configured for record class " + + recordClassName + ". Register a SpanSource for it in SpanCompositionPlugin."); + } + return source; + } + public static final String COLUMN_SOURCE_ID = "source_id"; public static final String COLUMN_PROJECT_ID = "project_id"; public static final String COLUMN_WDK_WEIGHT = "wdk_weight"; @@ -605,4 +641,30 @@ protected void readFeature(ResultSet resultSet, Feature feature, String suffix) feature.weight = resultSet.getInt("wdk_weight_" + suffix); feature.reversed = resultSet.getBoolean("is_reversed_" + suffix); } + + static class TranscriptSpanSource implements SpanSource { + @Override + public String createTableSql(String tableName, String[] region, String cacheSql) { + throw new UnsupportedOperationException("filled in by Task 3"); + } + } + + static class DynSpanSource implements SpanSource { + @Override + public String createTableSql(String tableName, String[] region, String cacheSql) { + throw new UnsupportedOperationException("filled in by Task 4"); + } + } + + static class VariantSpanSource implements SpanSource { + @Override + public String createTableSql(String tableName, String[] region, String cacheSql) { + throw new UnsupportedOperationException("filled in by Task 5"); + } + + @Override + public boolean isStrandless() { + return true; + } + } } diff --git a/WSFPlugin/src/test/java/org/apidb/apicomplexa/wsfplugin/spanlogic/SpanSourceTest.java b/WSFPlugin/src/test/java/org/apidb/apicomplexa/wsfplugin/spanlogic/SpanSourceTest.java new file mode 100644 index 00000000..72583117 --- /dev/null +++ b/WSFPlugin/src/test/java/org/apidb/apicomplexa/wsfplugin/spanlogic/SpanSourceTest.java @@ -0,0 +1,49 @@ +package org.apidb.apicomplexa.wsfplugin.spanlogic; + +import org.gusdb.wdk.model.WdkModelException; +import org.junit.Test; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; + +public class SpanSourceTest { + + @Test + public void unregisteredRecordClassThrowsNamingTheClass() { + try { + SpanCompositionPlugin.spanSourceFor("OrfRecordClasses.OrfRecordClass"); + fail("expected WdkModelException for an unregistered record class"); + } + catch (WdkModelException e) { + assertTrue("message should name the record class, was: " + e.getMessage(), + e.getMessage().contains("OrfRecordClasses.OrfRecordClass")); + } + } + + @Test + public void transcriptRecordClassIsRegistered() throws WdkModelException { + assertNotNull(SpanCompositionPlugin.spanSourceFor("TranscriptRecordClasses.TranscriptRecordClass")); + } + + @Test + public void dynSpanRecordClassIsRegistered() throws WdkModelException { + assertNotNull(SpanCompositionPlugin.spanSourceFor("DynSpanRecordClasses.DynSpanRecordClass")); + } + + @Test + public void variantRecordClassIsRegistered() throws WdkModelException { + assertNotNull(SpanCompositionPlugin.spanSourceFor("VariantRecordClasses.VariantRecordClass")); + } + + @Test + public void onlyVariantIsStrandless() throws WdkModelException { + assertEquals(false, + SpanCompositionPlugin.spanSourceFor("TranscriptRecordClasses.TranscriptRecordClass").isStrandless()); + assertEquals(false, + SpanCompositionPlugin.spanSourceFor("DynSpanRecordClasses.DynSpanRecordClass").isStrandless()); + assertEquals(true, + SpanCompositionPlugin.spanSourceFor("VariantRecordClasses.VariantRecordClass").isStrandless()); + } +} From 5f6d0685db7c92817f30aae289977d70e3521456 Mon Sep 17 00:00:00 2001 From: John Brestelli Date: Sat, 15 Aug 2026 12:13:06 -0400 Subject: [PATCH 02/10] Move transcript span SQL behind SpanSource, unchanged Co-Authored-By: Claude Opus 5 --- .../spanlogic/SpanCompositionPlugin.java | 13 +++++++- .../wsfplugin/spanlogic/SpanSourceTest.java | 31 +++++++++++++++++++ 2 files changed, 43 insertions(+), 1 deletion(-) diff --git a/WSFPlugin/src/main/java/org/apidb/apicomplexa/wsfplugin/spanlogic/SpanCompositionPlugin.java b/WSFPlugin/src/main/java/org/apidb/apicomplexa/wsfplugin/spanlogic/SpanCompositionPlugin.java index 56448abc..2645ebd7 100644 --- a/WSFPlugin/src/main/java/org/apidb/apicomplexa/wsfplugin/spanlogic/SpanCompositionPlugin.java +++ b/WSFPlugin/src/main/java/org/apidb/apicomplexa/wsfplugin/spanlogic/SpanCompositionPlugin.java @@ -645,7 +645,18 @@ protected void readFeature(ResultSet resultSet, Feature feature, String suffix) static class TranscriptSpanSource implements SpanSource { @Override public String createTableSql(String tableName, String[] region, String cacheSql) { - throw new UnsupportedOperationException("filled in by Task 3"); + StringBuilder builder = new StringBuilder(); + builder.append("CREATE TABLE " + tableName + " AS "); + builder.append("SELECT DISTINCT ca.source_id, ca.gene_source_id, "); + builder.append(" fl.sequence_source_id, fl.feature_type, "); + builder.append(" ca.wdk_weight, ca.project_id, "); + builder.append(" COALESCE(fl.is_reversed, 0) AS is_reversed, "); + builder.append(" " + region[0] + " AS begin, " + region[1] + " AS end "); + builder.append("FROM apidb.FeatureLocation fl, " + cacheSql + " ca "); + builder.append("WHERE fl.feature_source_id = ca.gene_source_id"); + builder.append(" AND fl.is_top_level = 1"); + builder.append(" AND fl.feature_type = 'GeneFeature'"); + return builder.toString(); } } diff --git a/WSFPlugin/src/test/java/org/apidb/apicomplexa/wsfplugin/spanlogic/SpanSourceTest.java b/WSFPlugin/src/test/java/org/apidb/apicomplexa/wsfplugin/spanlogic/SpanSourceTest.java index 72583117..257127de 100644 --- a/WSFPlugin/src/test/java/org/apidb/apicomplexa/wsfplugin/spanlogic/SpanSourceTest.java +++ b/WSFPlugin/src/test/java/org/apidb/apicomplexa/wsfplugin/spanlogic/SpanSourceTest.java @@ -46,4 +46,35 @@ public void onlyVariantIsStrandless() throws WdkModelException { assertEquals(true, SpanCompositionPlugin.spanSourceFor("VariantRecordClasses.VariantRecordClass").isStrandless()); } + + private static final String[] REGION = { "(CASE WHEN COALESCE(fl.is_reversed, 0) = 0 THEN (start_min + 1*(0)) END)", + "(CASE WHEN COALESCE(fl.is_reversed, 0) = 0 THEN (end_max + 1*(0)) END)" }; + private static final String CACHE = "(SELECT source_id, gene_source_id, project_id, wdk_weight FROM some_cache)"; + + private static String sqlFor(String recordClassName) throws WdkModelException { + return SpanCompositionPlugin.spanSourceFor(recordClassName).createTableSql("temp_1", REGION, CACHE); + } + + @Test + public void transcriptSourceJoinsOnGeneAndKeepsTopLevelFilter() throws WdkModelException { + String sql = sqlFor("TranscriptRecordClasses.TranscriptRecordClass"); + assertTrue(sql, sql.contains("FROM apidb.FeatureLocation fl")); + assertTrue(sql, sql.contains("fl.feature_source_id = ca.gene_source_id")); + assertTrue("is_top_level filter is load-bearing for PAR genes: " + sql, + sql.contains("fl.is_top_level = 1")); + assertTrue(sql, sql.contains("fl.feature_type = 'GeneFeature'")); + assertTrue(sql, sql.contains("CREATE TABLE temp_1 AS")); + } + + @Test + public void noSourceEmitsOracleOnlyConstructs() throws WdkModelException { + for (String rc : new String[] { "TranscriptRecordClasses.TranscriptRecordClass", + "DynSpanRecordClasses.DynSpanRecordClass", + "VariantRecordClasses.VariantRecordClass" }) { + String sql = sqlFor(rc); + assertTrue(rc + " must not use rownum: " + sql, !sql.contains("rownum")); + assertTrue(rc + " must not use DECODE: " + sql, !sql.contains("DECODE(")); + assertTrue(rc + " must alias its location table fl: " + sql, sql.contains(" fl,") || sql.contains(" fl ")); + } + } } From bda7a902f1c4167b7ecc6789484d1ff36276a375 Mon Sep 17 00:00:00 2001 From: John Brestelli Date: Sat, 15 Aug 2026 12:15:45 -0400 Subject: [PATCH 03/10] Port DynSpan span SQL to Postgres: DECODE -> CASE WHEN Also drops the synthetic is_top_level/feature_type columns, which existed only to satisfy filters the one-row-per-record builder no longer applies. Co-Authored-By: Claude Opus 5 --- .../spanlogic/SpanCompositionPlugin.java | 27 ++++++++++++++++++- .../wsfplugin/spanlogic/SpanSourceTest.java | 10 +++++++ 2 files changed, 36 insertions(+), 1 deletion(-) diff --git a/WSFPlugin/src/main/java/org/apidb/apicomplexa/wsfplugin/spanlogic/SpanCompositionPlugin.java b/WSFPlugin/src/main/java/org/apidb/apicomplexa/wsfplugin/spanlogic/SpanCompositionPlugin.java index 2645ebd7..d57e1c86 100644 --- a/WSFPlugin/src/main/java/org/apidb/apicomplexa/wsfplugin/spanlogic/SpanCompositionPlugin.java +++ b/WSFPlugin/src/main/java/org/apidb/apicomplexa/wsfplugin/spanlogic/SpanCompositionPlugin.java @@ -642,6 +642,25 @@ protected void readFeature(ResultSet resultSet, Feature feature, String suffix) feature.reversed = resultSet.getBoolean("is_reversed_" + suffix); } + /** + * Standard span table for a source that yields exactly one row per record. No + * is_top_level / feature_type filtering: that exists only to pick one row out of + * apidb.FeatureLocation, where a feature has several. + */ + private static String oneRowPerRecordSql(String tableName, String[] region, String locTable, + String cacheSql) { + StringBuilder builder = new StringBuilder(); + builder.append("CREATE TABLE " + tableName + " AS "); + builder.append("SELECT DISTINCT fl.feature_source_id AS source_id, 'dontcare' as gene_source_id, "); + builder.append(" fl.sequence_source_id, "); + builder.append(" ca.wdk_weight, ca.project_id, "); + builder.append(" COALESCE(fl.is_reversed, 0) AS is_reversed, "); + builder.append(" " + region[0] + " AS begin, " + region[1] + " AS end "); + builder.append("FROM " + locTable + " fl, " + cacheSql + " ca "); + builder.append("WHERE fl.feature_source_id = ca.source_id"); + return builder.toString(); + } + static class TranscriptSpanSource implements SpanSource { @Override public String createTableSql(String tableName, String[] region, String cacheSql) { @@ -663,7 +682,13 @@ public String createTableSql(String tableName, String[] region, String cacheSql) static class DynSpanSource implements SpanSource { @Override public String createTableSql(String tableName, String[] region, String cacheSql) { - throw new UnsupportedOperationException("filled in by Task 4"); + String locTable = "(SELECT source_id AS feature_source_id, project_id, " + + " regexp_substr(source_id, '[^:]+', 1, 1) as sequence_source_id, " + + " regexp_substr(regexp_substr(source_id, '[^:]+', 1, 2), '[^\\-]+', 1,1) as start_min, " + + " regexp_substr(regexp_substr(source_id, '[^:]+', 1, 2), '[^\\-]+', 1,2) as end_max, " + + " CASE WHEN regexp_substr(source_id, '[^:]+', 1, 3) = 'r' THEN 1 ELSE 0 END AS is_reversed " + + " FROM " + cacheSql + ")"; + return oneRowPerRecordSql(tableName, region, locTable, cacheSql); } } diff --git a/WSFPlugin/src/test/java/org/apidb/apicomplexa/wsfplugin/spanlogic/SpanSourceTest.java b/WSFPlugin/src/test/java/org/apidb/apicomplexa/wsfplugin/spanlogic/SpanSourceTest.java index 257127de..ec2e5656 100644 --- a/WSFPlugin/src/test/java/org/apidb/apicomplexa/wsfplugin/spanlogic/SpanSourceTest.java +++ b/WSFPlugin/src/test/java/org/apidb/apicomplexa/wsfplugin/spanlogic/SpanSourceTest.java @@ -77,4 +77,14 @@ public void noSourceEmitsOracleOnlyConstructs() throws WdkModelException { assertTrue(rc + " must alias its location table fl: " + sql, sql.contains(" fl,") || sql.contains(" fl ")); } } + + @Test + public void dynSpanSourceParsesCoordinatesWithoutOracleSyntax() throws WdkModelException { + String sql = sqlFor("DynSpanRecordClasses.DynSpanRecordClass"); + assertTrue(sql, sql.contains("CASE WHEN regexp_substr(source_id, '[^:]+', 1, 3) = 'r' THEN 1 ELSE 0 END AS is_reversed")); + assertTrue("coordinates come from the step's own cache table: " + sql, sql.contains(CACHE)); + assertTrue("one row per record, so no is_top_level: " + sql, !sql.contains("is_top_level")); + assertTrue("one row per record, so no feature_type: " + sql, !sql.contains("feature_type")); + assertTrue(sql, sql.contains("fl.feature_source_id = ca.source_id")); + } } From d093993cc489010765ea55a112b374011138e3fa Mon Sep 17 00:00:00 2001 From: John Brestelli Date: Sat, 15 Aug 2026 12:17:57 -0400 Subject: [PATCH 04/10] Add VariantSpanSource over ApidbTuning.VariationAttributes Co-Authored-By: Claude Opus 5 --- .../wsfplugin/spanlogic/SpanCompositionPlugin.java | 7 ++++++- .../wsfplugin/spanlogic/SpanSourceTest.java | 11 +++++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/WSFPlugin/src/main/java/org/apidb/apicomplexa/wsfplugin/spanlogic/SpanCompositionPlugin.java b/WSFPlugin/src/main/java/org/apidb/apicomplexa/wsfplugin/spanlogic/SpanCompositionPlugin.java index d57e1c86..1ee5a8f4 100644 --- a/WSFPlugin/src/main/java/org/apidb/apicomplexa/wsfplugin/spanlogic/SpanCompositionPlugin.java +++ b/WSFPlugin/src/main/java/org/apidb/apicomplexa/wsfplugin/spanlogic/SpanCompositionPlugin.java @@ -695,7 +695,12 @@ public String createTableSql(String tableName, String[] region, String cacheSql) static class VariantSpanSource implements SpanSource { @Override public String createTableSql(String tableName, String[] region, String cacheSql) { - throw new UnsupportedOperationException("filled in by Task 5"); + String locTable = "(SELECT va.source_id AS feature_source_id, va.project_id, " + + " va.sequence_source_id, " + + " va.location AS start_min, va.location AS end_max, " + + " 0 AS is_reversed " + + " FROM ApidbTuning.VariationAttributes va)"; + return oneRowPerRecordSql(tableName, region, locTable, cacheSql); } @Override diff --git a/WSFPlugin/src/test/java/org/apidb/apicomplexa/wsfplugin/spanlogic/SpanSourceTest.java b/WSFPlugin/src/test/java/org/apidb/apicomplexa/wsfplugin/spanlogic/SpanSourceTest.java index ec2e5656..4faac228 100644 --- a/WSFPlugin/src/test/java/org/apidb/apicomplexa/wsfplugin/spanlogic/SpanSourceTest.java +++ b/WSFPlugin/src/test/java/org/apidb/apicomplexa/wsfplugin/spanlogic/SpanSourceTest.java @@ -87,4 +87,15 @@ public void dynSpanSourceParsesCoordinatesWithoutOracleSyntax() throws WdkModelE assertTrue("one row per record, so no feature_type: " + sql, !sql.contains("feature_type")); assertTrue(sql, sql.contains("fl.feature_source_id = ca.source_id")); } + + @Test + public void variantSourceIsAZeroLengthFeatureFromVariationAttributes() throws WdkModelException { + String sql = sqlFor("VariantRecordClasses.VariantRecordClass"); + assertTrue(sql, sql.contains("FROM ApidbTuning.VariationAttributes va")); + assertTrue(sql, sql.contains("va.location AS start_min") && sql.contains("va.location AS end_max")); + assertTrue("a point feature has no strand: " + sql, sql.contains("0 AS is_reversed")); + assertTrue("project_id comes from the row, not the model: " + sql, sql.contains("va.project_id")); + assertTrue("one row per record, so no is_top_level: " + sql, !sql.contains("is_top_level")); + assertTrue(sql, sql.contains("fl.feature_source_id = ca.source_id")); + } } From 43534738b1878864732b5ff139d8acc053fc0dfd Mon Sep 17 00:00:00 2001 From: John Brestelli Date: Sat, 15 Aug 2026 12:25:57 -0400 Subject: [PATCH 05/10] Cast DynSpan coordinates to numeric; tighten span source tests regexp_substr returns text, and makeRegion does arithmetic on start_min/ end_max. Oracle coerced implicitly; PostgreSQL raises 'operator does not exist: text + integer', so segment colocation failed even after the DECODE fix. Text comparison would also have ordered spans lexicographically. Also: make the fl-alias assertion match the FROM clause instead of any occurrence, lowercase the Oracle-ism checks, and guard spanSourceFor(null). Co-Authored-By: Claude Opus 5 --- .../spanlogic/SpanCompositionPlugin.java | 15 +++++++++++-- .../wsfplugin/spanlogic/SpanSourceTest.java | 21 ++++++++++++++++--- 2 files changed, 31 insertions(+), 5 deletions(-) diff --git a/WSFPlugin/src/main/java/org/apidb/apicomplexa/wsfplugin/spanlogic/SpanCompositionPlugin.java b/WSFPlugin/src/main/java/org/apidb/apicomplexa/wsfplugin/spanlogic/SpanCompositionPlugin.java index 1ee5a8f4..fe5ffdff 100644 --- a/WSFPlugin/src/main/java/org/apidb/apicomplexa/wsfplugin/spanlogic/SpanCompositionPlugin.java +++ b/WSFPlugin/src/main/java/org/apidb/apicomplexa/wsfplugin/spanlogic/SpanCompositionPlugin.java @@ -129,6 +129,10 @@ default boolean isStrandless() { "VariantRecordClasses.VariantRecordClass", new VariantSpanSource()); static SpanSource spanSourceFor(String recordClassName) throws WdkModelException { + if (recordClassName == null) { + throw new WdkModelException("Genomic colocation is not configured for record class " + + "null. Register a SpanSource for it in SpanCompositionPlugin."); + } SpanSource source = SPAN_SOURCES.get(recordClassName); if (source == null) { throw new WdkModelException("Genomic colocation is not configured for record class " + @@ -682,10 +686,17 @@ public String createTableSql(String tableName, String[] region, String cacheSql) static class DynSpanSource implements SpanSource { @Override public String createTableSql(String tableName, String[] region, String cacheSql) { + // regexp_substr returns text on PostgreSQL, but makeRegion does arithmetic + // (start_min + n*(m)) on these columns. Oracle coerced text to number + // implicitly; PostgreSQL does not, so these must be cast explicitly. Cast + // to numeric (not integer) so this source's columns match the numeric + // start_min/end_max produced by the other SpanSource implementations, + // since composeSql compares begin/end values across temp tables built by + // different sources. String locTable = "(SELECT source_id AS feature_source_id, project_id, " + " regexp_substr(source_id, '[^:]+', 1, 1) as sequence_source_id, " + - " regexp_substr(regexp_substr(source_id, '[^:]+', 1, 2), '[^\\-]+', 1,1) as start_min, " + - " regexp_substr(regexp_substr(source_id, '[^:]+', 1, 2), '[^\\-]+', 1,2) as end_max, " + + " CAST(regexp_substr(regexp_substr(source_id, '[^:]+', 1, 2), '[^\\-]+', 1,1) AS numeric) as start_min, " + + " CAST(regexp_substr(regexp_substr(source_id, '[^:]+', 1, 2), '[^\\-]+', 1,2) AS numeric) as end_max, " + " CASE WHEN regexp_substr(source_id, '[^:]+', 1, 3) = 'r' THEN 1 ELSE 0 END AS is_reversed " + " FROM " + cacheSql + ")"; return oneRowPerRecordSql(tableName, region, locTable, cacheSql); diff --git a/WSFPlugin/src/test/java/org/apidb/apicomplexa/wsfplugin/spanlogic/SpanSourceTest.java b/WSFPlugin/src/test/java/org/apidb/apicomplexa/wsfplugin/spanlogic/SpanSourceTest.java index 4faac228..22cb5cef 100644 --- a/WSFPlugin/src/test/java/org/apidb/apicomplexa/wsfplugin/spanlogic/SpanSourceTest.java +++ b/WSFPlugin/src/test/java/org/apidb/apicomplexa/wsfplugin/spanlogic/SpanSourceTest.java @@ -72,12 +72,27 @@ public void noSourceEmitsOracleOnlyConstructs() throws WdkModelException { "DynSpanRecordClasses.DynSpanRecordClass", "VariantRecordClasses.VariantRecordClass" }) { String sql = sqlFor(rc); - assertTrue(rc + " must not use rownum: " + sql, !sql.contains("rownum")); - assertTrue(rc + " must not use DECODE: " + sql, !sql.contains("DECODE(")); - assertTrue(rc + " must alias its location table fl: " + sql, sql.contains(" fl,") || sql.contains(" fl ")); + String lowerSql = sql.toLowerCase(); + assertTrue(rc + " must not use rownum: " + sql, !lowerSql.contains("rownum")); + assertTrue(rc + " must not use DECODE: " + sql, !lowerSql.contains("decode(")); + assertTrue(rc + " must alias its location table fl in the FROM clause: " + sql, + sql.matches("(?s).*FROM .+ fl, .*")); } } + @Test + public void dynSpanCoordinatesAreNumericNotText() { + String sql; + try { + sql = sqlFor("DynSpanRecordClasses.DynSpanRecordClass"); + } + catch (WdkModelException e) { + throw new RuntimeException(e); + } + assertTrue("regexp_substr returns text; makeRegion does arithmetic on these: " + sql, + sql.contains("AS numeric) as start_min") && sql.contains("AS numeric) as end_max")); + } + @Test public void dynSpanSourceParsesCoordinatesWithoutOracleSyntax() throws WdkModelException { String sql = sqlFor("DynSpanRecordClasses.DynSpanRecordClass"); From fd98d0305e8d7d760a90202068879ced6ae48a52 Mon Sep 17 00:00:00 2001 From: John Brestelli Date: Sat, 15 Aug 2026 12:28:23 -0400 Subject: [PATCH 06/10] Dispatch span sources through the registry; drop dead Oracle and SNP paths - getSpanSql looks up a SpanSource instead of branching on record-class name - Flag.hasSnp -> Flag.strandless, named after the property not one record type - deletes the unreachable SnpRecordClass branch: every SNP import is commented out of apiCommonModel.xml, so that record class cannot exist at runtime - deletes the rownum subquery rather than porting it --- .../spanlogic/SpanCompositionPlugin.java | 70 ++----------------- 1 file changed, 7 insertions(+), 63 deletions(-) diff --git a/WSFPlugin/src/main/java/org/apidb/apicomplexa/wsfplugin/spanlogic/SpanCompositionPlugin.java b/WSFPlugin/src/main/java/org/apidb/apicomplexa/wsfplugin/spanlogic/SpanCompositionPlugin.java index fe5ffdff..c8725f3a 100644 --- a/WSFPlugin/src/main/java/org/apidb/apicomplexa/wsfplugin/spanlogic/SpanCompositionPlugin.java +++ b/WSFPlugin/src/main/java/org/apidb/apicomplexa/wsfplugin/spanlogic/SpanCompositionPlugin.java @@ -98,7 +98,8 @@ public String getRegion() { } private static class Flag { - private boolean hasSnp = false; + /** Set when either input is a point feature with no meaningful strand. */ + private boolean strandless = false; } /** @@ -422,7 +423,7 @@ private String composeSql(String operation, String tempTableA, String tempTableB builder.append(" AND fb.begin <= fb.end "); // check the strand choice - if (!flag.hasSnp) { + if (!flag.strandless) { if (strand.equalsIgnoreCase(PARAM_VALUE_SAME_STRAND)) { builder.append(" AND fa.is_reversed = fb.is_reversed "); } @@ -467,27 +468,9 @@ private String getSpanSql(WdkModel wdkModel, User requestingUser, Map Date: Sat, 15 Aug 2026 12:29:11 -0400 Subject: [PATCH 07/10] Refresh stale comment above the span source lookup --- .../apicomplexa/wsfplugin/spanlogic/SpanCompositionPlugin.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/WSFPlugin/src/main/java/org/apidb/apicomplexa/wsfplugin/spanlogic/SpanCompositionPlugin.java b/WSFPlugin/src/main/java/org/apidb/apicomplexa/wsfplugin/spanlogic/SpanCompositionPlugin.java index c8725f3a..c8d670eb 100644 --- a/WSFPlugin/src/main/java/org/apidb/apicomplexa/wsfplugin/spanlogic/SpanCompositionPlugin.java +++ b/WSFPlugin/src/main/java/org/apidb/apicomplexa/wsfplugin/spanlogic/SpanCompositionPlugin.java @@ -466,7 +466,7 @@ private String getSpanSql(WdkModel wdkModel, User requestingUser, Map Date: Sat, 15 Aug 2026 13:09:33 -0400 Subject: [PATCH 08/10] Name span temp tables bare in composeSql, not parenthesized Oracle accepts "FROM (table_name) alias"; PostgreSQL raises 'syntax error at or near ")"'. composeSql is shared by every colocation regardless of record type, so this broke ALL of them on Postgres -- including gene <-> gene, which the earlier analysis had assumed working. Found by live QA, not by the unit tests: they asserted on the per-source CREATE TABLE statements and never touched the join composed from them. composeSql is now package-private with a regression test. Co-Authored-By: Claude Opus 5 --- .../spanlogic/SpanCompositionPlugin.java | 13 +++++++++---- .../wsfplugin/spanlogic/SpanSourceTest.java | 15 +++++++++++++++ 2 files changed, 24 insertions(+), 4 deletions(-) diff --git a/WSFPlugin/src/main/java/org/apidb/apicomplexa/wsfplugin/spanlogic/SpanCompositionPlugin.java b/WSFPlugin/src/main/java/org/apidb/apicomplexa/wsfplugin/spanlogic/SpanCompositionPlugin.java index c8d670eb..884927d5 100644 --- a/WSFPlugin/src/main/java/org/apidb/apicomplexa/wsfplugin/spanlogic/SpanCompositionPlugin.java +++ b/WSFPlugin/src/main/java/org/apidb/apicomplexa/wsfplugin/spanlogic/SpanCompositionPlugin.java @@ -97,9 +97,9 @@ public String getRegion() { } - private static class Flag { + static class Flag { /** Set when either input is a point feature with no meaningful strand. */ - private boolean strandless = false; + boolean strandless = false; } /** @@ -396,7 +396,10 @@ private String[] getStartStop(Map params, String suffix) { return new String[] { start, stop }; } - private String composeSql(String operation, String tempTableA, String tempTableB, + // package-private so SpanSourceTest can assert the FROM clause names the temp tables + // bare. Wrapping a table name in parentheses is legal Oracle and a syntax error in + // PostgreSQL, which broke every colocation regardless of record type. + String composeSql(String operation, String tempTableA, String tempTableB, String strand, String output, Flag flag) { StringBuilder builder = new StringBuilder(); @@ -413,7 +416,9 @@ private String composeSql(String operation, String tempTableA, String tempTableB builder.append(" fb.wdk_weight AS wdk_weight_b, "); builder.append(" fb.begin AS begin_b, fb.end AS end_b, "); builder.append(" fb.is_reversed AS is_reversed_b "); - builder.append("FROM (" + tempTableA + ") fa, (" + tempTableB + ") fb "); + // Bare table names, NOT "(name)". Oracle accepts a parenthesized table name; + // PostgreSQL raises 'syntax error at or near ")"'. + builder.append("FROM " + tempTableA + " fa, " + tempTableB + " fb "); // make sure the regions come from sequence source. builder.append("WHERE fa.sequence_source_id = fb.sequence_source_id "); diff --git a/WSFPlugin/src/test/java/org/apidb/apicomplexa/wsfplugin/spanlogic/SpanSourceTest.java b/WSFPlugin/src/test/java/org/apidb/apicomplexa/wsfplugin/spanlogic/SpanSourceTest.java index 22cb5cef..6c8dc828 100644 --- a/WSFPlugin/src/test/java/org/apidb/apicomplexa/wsfplugin/spanlogic/SpanSourceTest.java +++ b/WSFPlugin/src/test/java/org/apidb/apicomplexa/wsfplugin/spanlogic/SpanSourceTest.java @@ -113,4 +113,19 @@ public void variantSourceIsAZeroLengthFeatureFromVariationAttributes() throws Wd assertTrue("one row per record, so no is_top_level: " + sql, !sql.contains("is_top_level")); assertTrue(sql, sql.contains("fl.feature_source_id = ca.source_id")); } + + /** + * Oracle accepts "FROM (table_name) alias"; PostgreSQL raises + * 'syntax error at or near ")"'. This broke EVERY colocation, not just the new + * record types, because composeSql is shared by all of them. + */ + @Test + public void composeSqlNamesTempTablesBare() { + String sql = new SpanCompositionPlugin().composeSql("overlap", "spanlogic_a", "spanlogic_b", + "either_strand", "a", new SpanCompositionPlugin.Flag()); + assertTrue("temp tables must not be parenthesized: " + sql, + sql.contains("FROM spanlogic_a fa, spanlogic_b fb")); + assertTrue("a parenthesized table name is an Oracle-ism: " + sql, + !sql.contains("(spanlogic_a)") && !sql.contains("(spanlogic_b)")); + } } From c91cbbe3300c8cfc2c4714d6547082fa522388a3 Mon Sep 17 00:00:00 2001 From: John Brestelli Date: Sat, 15 Aug 2026 13:19:19 -0400 Subject: [PATCH 09/10] Drop span temp tables unqualified, matching the unqualified CREATE getDefaultSchema() means different things per platform: Oracle returns the login user's schema -- exactly where an unqualified CREATE TABLE lands, so create and drop agreed. PostgreSQL hardcodes "public", while an unqualified CREATE follows search_path ("$user"). So the temp tables were created in the login schema and the drop looked in public, failing with 'table "spanlogic" does not exist' AFTER the results were computed. A working colocation surfaced to the user as an error, and leaked one table per run (38 found in the appDb). Passing null makes dropTable emit a bare table name, resolving the same way the CREATE did, on either platform. Co-Authored-By: Claude Opus 5 --- .../spanlogic/SpanCompositionPlugin.java | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/WSFPlugin/src/main/java/org/apidb/apicomplexa/wsfplugin/spanlogic/SpanCompositionPlugin.java b/WSFPlugin/src/main/java/org/apidb/apicomplexa/wsfplugin/spanlogic/SpanCompositionPlugin.java index 884927d5..f77eb2d6 100644 --- a/WSFPlugin/src/main/java/org/apidb/apicomplexa/wsfplugin/spanlogic/SpanCompositionPlugin.java +++ b/WSFPlugin/src/main/java/org/apidb/apicomplexa/wsfplugin/spanlogic/SpanCompositionPlugin.java @@ -322,12 +322,20 @@ public int execute(PluginRequest request, PluginResponse response) throws Plugin // execute the final sql, and fetch the result for the output. prepareResult(wdkModel, response, sql, request.getOrderedColumns(), output); - // drop the cache tables + // Drop the cache tables UNQUALIFIED, to match the unqualified CREATE TABLE in + // getSpanSql. Do not reach for getDefaultSchema() here: it means different things + // per platform. Oracle returns the login user's schema -- which is exactly where an + // unqualified CREATE lands, so the two agreed. PostgreSQL hardcodes "public" + // (PostgreSQL.getDefaultSchema), while an unqualified CREATE follows search_path, + // which is "$user". The tables were therefore created in the login schema and the + // drop looked in public, failing with 'table "spanlogic" does not exist' AFTER + // the results had been computed -- so a working colocation surfaced as an error and + // leaked a table per run. Passing null makes dropTable emit a bare table name, + // which resolves the same way the CREATE did on either platform. DBPlatform platform = wdkModel.getAppDb().getPlatform(); DataSource dataSource = wdkModel.getAppDb().getDataSource(); - String schema = wdkModel.getAppDb().getDefaultSchema(); - platform.dropTable(dataSource, schema, tempA, true); - platform.dropTable(dataSource, schema, tempB, true); + platform.dropTable(dataSource, null, tempA, true); + platform.dropTable(dataSource, null, tempB, true); return 0; } From f9d3fdf0bd0c89357f9c94ae922b5a78fa0451f4 Mon Sep 17 00:00:00 2001 From: John Brestelli Date: Mon, 17 Aug 2026 10:01:02 -0400 Subject: [PATCH 10/10] Fix comments naming a method that does not exist The SpanSource javadoc and the DynSpan cast comment both cited "makeRegion()" as the code that hardcodes the "fl." prefix and does arithmetic on start_min/end_max. No such method exists -- it is getStartStop() (SpanCompositionPlugin.java:351, "String table = fl."). The wrong name was introduced with the comments themselves, so it never resolved for anyone following it. Caught in review by @steve-fischer-200. Co-Authored-By: Claude Opus 5 --- .../wsfplugin/spanlogic/SpanCompositionPlugin.java | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/WSFPlugin/src/main/java/org/apidb/apicomplexa/wsfplugin/spanlogic/SpanCompositionPlugin.java b/WSFPlugin/src/main/java/org/apidb/apicomplexa/wsfplugin/spanlogic/SpanCompositionPlugin.java index f77eb2d6..972ea7e3 100644 --- a/WSFPlugin/src/main/java/org/apidb/apicomplexa/wsfplugin/spanlogic/SpanCompositionPlugin.java +++ b/WSFPlugin/src/main/java/org/apidb/apicomplexa/wsfplugin/spanlogic/SpanCompositionPlugin.java @@ -106,8 +106,9 @@ static class Flag { * Where a record type's genomic coordinates come from. One implementation per record * class that may be an input to colocation. * - * Implementations MUST alias their location table "fl" -- makeRegion() hardcodes that - * prefix when building the region expressions interpolated into every builder. + * Implementations MUST alias their location table "fl" -- getStartStop() hardcodes that + * prefix (String table = "fl.") when building the region[] expressions that every + * implementation interpolates into its SQL. */ interface SpanSource { @@ -643,7 +644,7 @@ public String createTableSql(String tableName, String[] region, String cacheSql) static class DynSpanSource implements SpanSource { @Override public String createTableSql(String tableName, String[] region, String cacheSql) { - // regexp_substr returns text on PostgreSQL, but makeRegion does arithmetic + // regexp_substr returns text on PostgreSQL, but getStartStop does arithmetic // (start_min + n*(m)) on these columns. Oracle coerced text to number // implicitly; PostgreSQL does not, so these must be cast explicitly. Cast // to numeric (not integer) so this source's columns match the numeric