From 67e4ac36d6525b6867710487d18a725ead34766c Mon Sep 17 00:00:00 2001 From: loi Date: Wed, 15 Jul 2026 09:59:51 -0700 Subject: [PATCH] FIREFLY-1923: Fix inability to load large FITS tables - FITSTableReader: read columns via getColumn() instead of row-by-row getRow(), resolving type dispatch once per column instead of per cell - deg2pix: scrub NaN, clamp jp/jm to [0, nside) on both bounds (fixes left-shift overflow) - decimate_key/decimation stats: guard NaN and +/-Infinity, not just NaN FIREFLY-2047: Improve decimation handling of non-finite values - throw a clear error when a column has no finite values - exclude merged invalid-rows bucket from decimated output --- .../ipac/firefly/resources/healpix-java.sql | 32 ++++++++-- .../ipac/firefly/server/db/DuckDbUDF.java | 4 +- .../server/query/DecimationProcessor.java | 13 +++- .../ipac/table/io/FITSTableReader.java | 61 +++++++++++++------ 4 files changed, 84 insertions(+), 26 deletions(-) diff --git a/src/firefly/java/edu/caltech/ipac/firefly/resources/healpix-java.sql b/src/firefly/java/edu/caltech/ipac/firefly/resources/healpix-java.sql index ca8296b518..a0c4445991 100644 --- a/src/firefly/java/edu/caltech/ipac/firefly/resources/healpix-java.sql +++ b/src/firefly/java/edu/caltech/ipac/firefly/resources/healpix-java.sql @@ -16,6 +16,23 @@ AS -- java's >>> unsigned bitwise right shift for int64 CREATE OR REPLACE FUNCTION uRightShift(v, n) AS (v >> n) & ((1::LONG << (64 - n)) - 1); +-- unlike built-in LEAST()/GREATEST(), these return NULL instead of skipping it. +CREATE OR REPLACE FUNCTION least_null(a, b) +AS ( + CASE WHEN a IS NULL OR b IS NULL THEN NULL + WHEN a < b THEN a + ELSE b + END +); + +CREATE OR REPLACE FUNCTION greatest_null(a, b) +AS ( + CASE WHEN a IS NULL OR b IS NULL THEN NULL + WHEN a > b THEN a + ELSE b + END +); + -- Return remainder of the division v1/v2; positive and smaller than v2 -- v1 dividend; can be positive or negative -- v2 divisor; must be positive @@ -117,9 +134,12 @@ AS ( END AS tmp ), p_f AS ( + -- clamp to [0, nside-1] on both ends: tp/tmp can land outside this range when + -- a_ra/a_dec are outside the physical ra/dec domain (e.g. non ra/dec columns + -- used as if they were coordinates) SELECT - LEAST(TRUNC(tp * tmp)::LONG, nside - 1) AS jp, - LEAST(TRUNC((1.0 - tp) * tmp)::LONG, nside - 1) AS jm, + greatest_null(0, least_null(TRUNC(tp * tmp)::LONG, nside - 1)) AS jp, + greatest_null(0, least_null(TRUNC((1.0 - tp) * tmp)::LONG, nside - 1)) AS jm, FROM p_1 ) SELECT @@ -135,9 +155,11 @@ AS ( CREATE OR REPLACE FUNCTION deg2pix(n_order, a_ra, a_dec) AS ( WITH step_1 AS ( + -- replace NaN with NULL up front: NULL propagates safely through the casts + -- below, but NaN cannot be cast to LONG which causes query to fail. SELECT - radians(90 - a_dec) AS theta, - adj_phi(radians(a_ra)) AS phi, + radians(90 - (CASE WHEN isnan(a_dec) THEN NULL ELSE a_dec END)) AS theta, + adj_phi(radians(CASE WHEN isnan(a_ra) THEN NULL ELSE a_ra END)) AS phi, ), step_2 AS ( SELECT @@ -171,7 +193,7 @@ AS ( WHEN ((2.0/3) >= za) THEN pix_equatorial(n_order, nside, tt, z) ELSE - pix_polar(n_order, nside, have_sth, sth, tt, LEAST(3, TRUNC(tt)::LONG), z, za) + pix_polar(n_order, nside, have_sth, sth, tt, least_null(3, TRUNC(tt)::LONG), z, za) END AS pixel FROM step_f ); diff --git a/src/firefly/java/edu/caltech/ipac/firefly/server/db/DuckDbUDF.java b/src/firefly/java/edu/caltech/ipac/firefly/server/db/DuckDbUDF.java index 73c96a5cf2..ec5a34f7d2 100644 --- a/src/firefly/java/edu/caltech/ipac/firefly/server/db/DuckDbUDF.java +++ b/src/firefly/java/edu/caltech/ipac/firefly/server/db/DuckDbUDF.java @@ -18,7 +18,9 @@ public class DuckDbUDF { public static String deg2pix = importDeg2Pix(); public static final String decimate_key = """ CREATE OR REPLACE FUNCTION decimate_key(xVal, yVal, xMin, yMin, nX, nY, xUnit, yUnit) AS - TRUNC((xVal-xMin)/xUnit)::INT || ':' || TRUNC((yVal-yMin)/yUnit)::INT + CASE WHEN NOT isfinite(xVal) OR NOT isfinite(yVal) THEN NULL + ELSE TRUNC((xVal-xMin)/xUnit)::INT || ':' || TRUNC((yVal-yMin)/yUnit)::INT + END """; // make sure this matches DecimateKey.getKey() public static final String lg = "CREATE OR REPLACE FUNCTION lg(val) AS LOG10(val)"; public static final String nvl2 = """ diff --git a/src/firefly/java/edu/caltech/ipac/firefly/server/query/DecimationProcessor.java b/src/firefly/java/edu/caltech/ipac/firefly/server/query/DecimationProcessor.java index da52028074..570423fe0d 100644 --- a/src/firefly/java/edu/caltech/ipac/firefly/server/query/DecimationProcessor.java +++ b/src/firefly/java/edu/caltech/ipac/firefly/server/query/DecimationProcessor.java @@ -95,7 +95,7 @@ void createDecimateTable(TableServerRequest treq, DbAdapter dbAdapter) throws Da count(*) as "weight", %s as "dkey" FROM %s GROUP BY "dkey" - ) WHERE "%s" IS NOT NULL AND "%s" IS NOT NULL + ) WHERE "%s" IS NOT NULL AND "%s" IS NOT NULL AND "dkey" IS NOT NULL ) """.formatted(tblName, ROW_NUM, ROW_IDX, deciInfo.getxExp(), deciKey.getXCol(), deciInfo.getyExp(), deciKey.getYCol(), deciFunc, dataTbl, deciKey.getXCol(), deciKey.getYCol()); dbAdapter.execUpdate(sql); @@ -157,8 +157,10 @@ public static void setDecimateInfo(ServerRequest req, DecimateInfo decimateInfo) } public static DecimateKey getDeciKey(DecimateInfo deciInfo, DbAdapter dbAdapter, String tblName) throws DataAccessException { + // exclude NaN and +/-Infinity from the stats: either can poison MAX/MIN. String sql = """ - SELECT MAX(%1$s) as "xMax", MIN(%1$s) as "xMin", COUNT(%1$s) as "xCount", MAX(%2$s) as "yMax", MIN(%2$s) as "yMin", COUNT(%2$s) as "yCount" from %3$s + SELECT MAX(x) as "xMax", MIN(x) as "xMin", COUNT(x) as "xCount", MAX(y) as "yMax", MIN(y) as "yMin", COUNT(y) as "yCount" + FROM (SELECT CASE WHEN NOT isfinite(%1$s) THEN NULL ELSE %1$s END as x, CASE WHEN NOT isfinite(%2$s) THEN NULL ELSE %2$s END as y FROM %3$s) """.formatted(deciInfo.getxExp(), deciInfo.getyExp(), tblName); DataGroup stats = dbAdapter.execQuery(sql, null); @@ -170,6 +172,13 @@ SELECT MAX(%1$s) as "xMax", MIN(%1$s) as "xMin", COUNT(%1$s) as "xCount", MAX(%2 int xCount = toInt(stats.getData("xCount", 0)); int yCount = toInt(stats.getData("yCount", 0)); + // a column with zero finite values makes MAX/MIN come back SQL NULL, which toDouble() + // turns into NaN; + if (xCount == 0 || yCount == 0) { + String badCol = xCount == 0 ? deciInfo.getxColumnName() : deciInfo.getyColumnName(); + throw new DataAccessException("Unable to decimate: column \"%s\" has no valid (finite) values".formatted(badCol)); + } + var deciKey = getDecimateKey(deciInfo, xMax, xMin, yMax, yMin); deciKey.setxCount(xCount); deciKey.setyCount(yCount); diff --git a/src/firefly/java/edu/caltech/ipac/table/io/FITSTableReader.java b/src/firefly/java/edu/caltech/ipac/table/io/FITSTableReader.java index 95f7404dbd..f16e39f46a 100644 --- a/src/firefly/java/edu/caltech/ipac/table/io/FITSTableReader.java +++ b/src/firefly/java/edu/caltech/ipac/table/io/FITSTableReader.java @@ -435,17 +435,31 @@ private static void doIngestFitsTable(TableParseHandler handler, var dataGroup= fitsTableReadInfo.dataGroup(); var evaluator= fitsTableReadInfo.evaluator(); int totalRows = hduTable.getNRows(); + int nCol = hduTable.getNCols(); SpectrumMetaInspector.searchForSpectrum(dataGroup,hduTable, spectrumHint); - DataType[] dataDefinitions= dataGroup.getDataDefinitions(); dataGroup.trimToSize(); handler.start(); handler.startTable(0); handler.header(dataGroup); - hduTable.getKernel(); - for (int row = 0; row < totalRows; row++){ - ingestRow(handler, dataGroup, dataDefinitions, row, hduTable, evaluator); + // makeRowExtractor() resolves getValAsObject()'s reflection-based dispatch once per + // column instead of once per cell (nRow x nCol times). + RowExtractor[] extractors = new RowExtractor[nCol]; + for (int icol = 0; icol < nCol; icol++) { + extractors[icol] = makeRowExtractor(hduTable.getColumn(icol), evaluator[icol]); + } + + for (int row = 0; row < totalRows; row++) { + try { + Object[] outRow = new Object[nCol]; + for (int icol = 0; icol < nCol; icol++) { + outRow[icol] = extractors[icol].get(row); + } + handler.data(outRow); + } catch (Exception e) { + logger.error("Unable to read table row:" + row + " msg:" + e.getMessage()); + } } } catch (Exception e) { throw new IOException(e.toString(),e); @@ -567,6 +581,31 @@ public Number evalValue(Number val) { } } + @FunctionalInterface + private interface RowExtractor { + Object get(int row); + } + + /** + * Resolves how to pull a single row's value out of a whole-column array returned by + * {@link TableHDU#getColumn}, once per column rather than once per cell. + * scale/blank handling below mirrors getValAsObject(). + */ + private static RowExtractor makeRowExtractor(Object col, EvalVal ev) { + return switch (col) { + case double[] a -> row -> ev.evalValue(a[row]); + case float[] a -> row -> ev.evalValue(a[row]); + case int[] a -> row -> ev.evalValue(a[row]); + case long[] a -> row -> ev.evalValue(a[row]); + case short[] a -> row -> ev.evalValue((int) a[row]); + case byte[] a -> row -> ev.evalValue((int) a[row]); + case boolean[] a -> row -> a[row]; + case String[] a -> row -> isEmpty(a[row]) ? null : a[row]; + case Object[] a -> row -> ArrayFuncs.flatten(a[row]); // vector-valued column (repeat > 1) + default -> throw new IllegalStateException("Unrecognized FITS column type: " + col.getClass()); + }; + } + //This function is loosely based on the packageValue function from the FitsStarTable class in the uk.ac.starlink.fits package private static Object getValAsObject(Object elem, int icol, EvalVal[] evaluator) throws FitsException { if (elem == null) { @@ -598,20 +637,6 @@ else if (Array.getLength(elem) == 1) { - private static void ingestRow(TableParseHandler handler, DataGroup dataGroup, DataType[] dataDefinitions, - int rowIdx, TableHDU hduTable, EvalVal[] evaluator) throws FitsException { - Object[] outRow= new Object[dataDefinitions.length]; - try { - Object[] rowData= hduTable.getRow(rowIdx); - for (int dtIdx = 0; dtIdx < dataGroup.getDataDefinitions().length; dtIdx++) { - //so cast the val object to an array of its type by calling the getValAsObject function - outRow[dtIdx] = getValAsObject(rowData[dtIdx], dtIdx, evaluator); - } - handler.data(outRow); - } catch (Exception e) { - logger.error("Unable to read table row:" + rowIdx + " msg:" + e.getMessage()); - } - }