FIREFLY-49: Table filter shows 'null' in the column category but fails to filter

src/firefly/html/test/tests-main.html

@@ -18,10 +18,10 @@
 
 <template title="IRSA Gator Table" class="tpl" >
     <div id="expected" style="position: relative" >
-        <img style="position: absolute; right: 2px; width: 275px" src="./images/gator-expected.jpg"/>
+        <img style="position: absolute; left: 2px" src="./images/gator-expected.jpg"/>
         <div class="source-code indent-3" style="position: absolute; bottom: 0; left: 0">
+            - toolbars matches above
             - widgets loaded
-            - toolbars match
         </div>
     </div>
     <div id="actual" class="flow-h">

src/firefly/java/edu/caltech/ipac/firefly/server/db/BaseDbAdapter.java

@@ -9,7 +9,6 @@
 import edu.caltech.ipac.firefly.server.util.Logger;
 import edu.caltech.ipac.table.DataType;
 import edu.caltech.ipac.util.StringUtils;
-import org.apache.xpath.operations.Bool;
 
 import java.io.File;
 import java.util.ArrayList;
@@ -19,6 +18,7 @@
 import java.util.List;
 import java.util.Map;
 import java.util.concurrent.ConcurrentHashMap;
+import java.util.regex.Pattern;
 import java.util.stream.Collectors;
 
 import static edu.caltech.ipac.firefly.data.TableServerRequest.INCL_COLUMNS;
@@ -134,10 +134,14 @@ public String wherePart(TableServerRequest treq) {
         if (treq.getFilters() != null && treq.getFilters().size() > 0) {
             where = "";
             for (String cond :treq.getFilters()) {
-                if (cond.matches("(?i).* LIKE .*(\\\\_|\\\\%|\\\\\\\\).*")) {       // search for LIKE w/  \_, \%, or \\ in the condition.
+                if (cond.matches("(?i).* LIKE .*(\\\\_|\\\\%|\\\\\\\\).*")) {       // search for LIKE with  \_, \%, or \\ in the condition.
                     // for LIKE, to search for '%', '\' or '_' itself, an escape character must also be specified using the ESCAPE clause
                     cond += " ESCAPE '\\'";
                 }
+                String[] parts = StringUtils.groupMatch("(.+) IN (.+)", cond, Pattern.CASE_INSENSITIVE);
+                if (parts != null && cond.contains(NULL_TOKEN)) {
+                    cond = String.format("%s OR %s IS NULL", cond.replace(NULL_TOKEN, NULL_TOKEN.substring(1)), parts[0]);
+                }
                 if (where.length() > 0) {
                     where += " and ";
                 }

src/firefly/java/edu/caltech/ipac/firefly/server/db/DbAdapter.java

@@ -24,6 +24,7 @@ public interface DbAdapter {
     String HSQL = "hsql";
 
     String MAIN_DB_TBL = "DATA";
+    String NULL_TOKEN = "%NULL";
 
     /*
       CLEAN UP POLICY:

src/firefly/java/edu/caltech/ipac/firefly/server/db/EmbeddedDbUtil.java

@@ -433,7 +433,7 @@ private static int dbToDD(DataGroup dg, ResultSet rs) {
 
                 applyIfNotEmpty(rs.getString("label"), dtype::setLabel);
                 applyIfNotEmpty(rs.getString("units"), dtype::setUnits);
-                applyIfNotEmpty(rs.getString("null_str"), dtype::setNullString);
+                dtype.setNullString(rs.getString("null_str"));
                 applyIfNotEmpty(rs.getString("format"), dtype::setFormat);
                 applyIfNotEmpty(rs.getString("fmtDisp"), dtype::setFmtDisp);
                 applyIfNotEmpty(rs.getInt("width"), dtype::setWidth);

src/firefly/java/edu/caltech/ipac/firefly/server/query/AsyncSearchProcessor.java

@@ -33,6 +33,7 @@ public DataGroup fetchDataGroup(TableServerRequest req) throws DataAccessExcepti
                     case COMPLETED:
                         return asyncJob.getDataGroup();
                     case ERROR:
+                    case UNKNOWN:
                         throw new DataAccessException(asyncJob.getErrorMsg());
                     case ABORTED:
                         throw new DataAccessException("Query aborted");

src/firefly/java/edu/caltech/ipac/firefly/server/query/EmbeddedDbProcessor.java

@@ -55,6 +55,7 @@
 import static edu.caltech.ipac.firefly.data.table.MetaConst.HIGHLIGHTED_ROW_BY_ROWIDX;
 import static edu.caltech.ipac.firefly.server.ServerContext.SHORT_TASK_EXEC;
 import static edu.caltech.ipac.firefly.server.db.DbAdapter.MAIN_DB_TBL;
+import static edu.caltech.ipac.firefly.server.db.DbAdapter.NULL_TOKEN;
 import static edu.caltech.ipac.firefly.server.db.EmbeddedDbUtil.execRequestQuery;
 import static edu.caltech.ipac.util.StringUtils.isEmpty;
 
@@ -586,9 +587,8 @@ private static void enumeratedValuesCheck(File dbFile, DataGroupPart results, Ta
                             .queryForList(String.format("SELECT distinct \"%s\" FROM data order by 1", cname));
 
                     if (vals.size() <= MAX_COL_ENUM_COUNT) {
-                        DataType dt = results.getData().getDataDefintion(cname);
-                        String enumVals = vals.stream().map(m -> String.valueOf(m.get(cname)))       // list of map to list of string(colname)
-                                .filter(s -> !isEmpty(s) && !StringUtils.areEqual(dt.getNullString(), s))            // remove null or blank values because it's hard to handle at the column filter level
+                        String enumVals = vals.stream()
+                                .map(m -> m.get(cname) == null ? NULL_TOKEN : m.get(cname).toString())  // list of map to list of string(colname)
                                 .collect(Collectors.joining(","));                          // combine the names into comma separated string.
                         results.getData().getDataDefintion(cname).setEnumVals(enumVals);
                         // update dd table

src/firefly/java/edu/caltech/ipac/table/DataType.java

@@ -52,7 +52,7 @@ public enum Visibility {show, hide, hidden};
     private       String typeDesc;
     private       Class type;
     private       String units;
-    private       String nullString = "";
+    private       String nullString;
     private       String desc;
     private       int width;
     private       int prefWidth;
@@ -142,7 +142,7 @@ public void setUnits(String units) {
     }
 
     public String getNullString() {
-        return nullString;
+        return nullString == null ? "" : nullString;
     }
 
     public void setNullString(String nullString) {
@@ -305,9 +305,8 @@ public String getRef() {
      * @return
      */
     public String format(Object value, boolean replaceCtrl) {
-        if (value == null) {
-            return getNullString() == null ? "" : getNullString();
-        }
+        if (value == null) return getNullString();
+
         // do escaping if requested
         if (replaceCtrl && type == String.class) {
             value = replaceCtrl((String)value);
@@ -419,8 +418,7 @@ public boolean isKnownType() {
      * @return an object
      */
     public Object convertStringToData(String s) {
-        if (s == null || s.length() == 0 || s.equalsIgnoreCase("null")) return null;
-        if (nullString != null && nullString.equals(s)) return null;
+        if (s == null || getNullString().equals(s)) return null;
 
         Object retval= s;
         try {

src/firefly/java/edu/caltech/ipac/table/IpacTableUtil.java

@@ -246,6 +246,7 @@ public static IpacTableDef createColumnDefs(String line) {
             for (int idx = 0; idx < names.length; idx++) {
                 cname = names[idx];
                 DataType dt = new DataType(cname.trim(), null);
+                dt.setNullString("null");       // defaults to 'null'
                 cols.add(dt);
                 tableDef.setColOffsets(idx, cursor);
                 cursor += cname.length() + 1;

src/firefly/java/edu/caltech/ipac/util/StringUtils.java

@@ -74,7 +74,8 @@ public static String[] groupMatch(String regex, String val) {
      * an array of strings, otherwise null.
      * @param regex pattern to match
      * @param val   string value to match with
-     * @return
+     * @param flags Match flags, a bit mask
+     * @return return all of the matching groups as an array of strings, otherwise null.
      */
     public static String[] groupMatch(String regex, String val, int flags) {
         return groupMatch(Pattern.compile(regex, flags), val);

src/firefly/js/tables/FilterInfo.js

@@ -4,7 +4,7 @@
 
 import {getColumnIdx, getColumn, isNumericType, getTblById, stripColumnNameQuotes} from './TableUtil.js';
 import {Expression} from '../util/expr/Expression.js';
-import {isUndefined, get, isArray, isEmpty} from 'lodash';
+import {isNil, get, isArray, isEmpty} from 'lodash';
 import {showInfoPopup} from '../ui/PopupUtil.jsx';
 
 const operators = /(!=|>=|<=|<|>|=| like | in | is not | is )/i;
@@ -23,6 +23,8 @@ export const FILTER_TTIPS =
 Examples:  "ra" > 12345; "color" != 'blue'; "band" IN (1,2,3)
 `;
 
+export const NULL_TOKEN = '%NULL';          // need to match DbAdapter.NULL_TOKEN
+
 /**
  * return [column_name, operator, value] triplet.
  * @param {string} input
@@ -191,7 +193,7 @@ export class FilterInfo {
 
         // remove the double quote or the single quote around cname and val (which is added in auto-correction)
         const removeQuoteAroundString = (str, quote = "'") => {
-            if (str.startsWith(quote)) {
+            if (str && str.startsWith(quote)) {
                 const reg = new RegExp('^' + quote + '(.*)' + quote + '$');
                 return str.replace(reg, '$1');
             } else {
@@ -223,7 +225,7 @@ export class FilterInfo {
             if (val.match(/^\(.*\)$/)) {
                 val = val.substring(1, val.length-1);
             }
-            val = val.split(',').map((s) => s.trim());
+            val = val.split(',').map((s) => removeQuoteAroundString(s.trim()) === NULL_TOKEN.toLowerCase() ? null : s.trim());
         }
 
         // remove single quote enclosing the string for the value of operater 'like' or char type column
@@ -236,12 +238,12 @@ export class FilterInfo {
         return (row, idx) => {
             if (!row) return false;
             let compareTo = noROWID ? idx : row[cidx];
-            if (isUndefined(compareTo)) return false;
+            compareTo = (compareTo === get(getColumn(tableModel, cname), 'nullString', '')) ? null : compareTo;     // resolve nullString
 
             if (op !== 'like' && colType.match(/^[dfil]/)) {      // int, float, double, long .. or their short form.
-                compareTo = Number(compareTo);
+                compareTo = compareTo ? Number(compareTo) : compareTo;
             } else {
-                compareTo = compareTo.toLowerCase();
+                compareTo = compareTo ? compareTo.toLowerCase() : compareTo;
             }
 
             switch (op) {
@@ -262,6 +264,10 @@ export class FilterInfo {
                     return compareTo <= val;
                 case 'in'  :
                     return val.includes(compareTo);
+                case 'is'  :
+                    return val === 'null' && isNil(compareTo);
+                case 'is not'  :
+                    return val === 'null' && !isNil(compareTo);
                 default :
                     return false;
             }
@@ -440,7 +446,7 @@ function autoCorrectCondition(v, isNumeric=false) {
     switch (op) {
         case 'like':
             if (!val.match(/^'.*'$/)) {
-                val = val.replace(/([_|%|\\])/g, '\\$1');
+                val = val.replace(/([_|%\\])/g, '\\$1');
 
                 val = encloseByQuote(encloseByQuote(val, '%'));
             }

src/firefly/js/tables/TableUtil.js

@@ -533,12 +533,16 @@ export function sortTableData(tableData, columns, sortInfoStr) {
     var comparator;
     if (!col.type || ['char', 'c'].includes(col.type) ) {
         comparator = (r1, r2) => {
-            const [s1, s2] = [r1[colIdx], r2[colIdx]];
+            let [s1, s2] = [r1[colIdx], r2[colIdx]];
+            s1 = s1 === '' ? '\u0002' : s1 === null ? '\u0001' : isUndefined(s1) ? '\u0000' : s1;
+            s2 = s2 === '' ? '\u0002' : s2 === null ? '\u0001' : isUndefined(s2) ? '\u0000' : s2;
             return multiplier * (s1 > s2 ? 1 : -1);
         };
     } else {
         comparator = (r1, r2) => {
-            const [v1, v2] = [r1[colIdx], r2[colIdx]];
+            let [v1, v2] = [r1[colIdx], r2[colIdx]];
+            v1 = v1 === null ? -Number.MAX_VALUE : isUndefined(v1) ? Number.NEGATIVE_INFINITY : Number(v1);
+            v2 = v2 === null ? -Number.MAX_VALUE : isUndefined(v2) ? Number.NEGATIVE_INFINITY : Number(v2);
             return multiplier * (Number(v1) - Number(v2));
         };
     }
@@ -895,8 +899,13 @@ export function tableDetailsView(tbl_id, highlightedRow, details_tbl_id) {
  */
 export function calcColumnWidths(columns, dataAry) {
     return columns.map( (cv, idx) => {
+
+        let width = cv.prefWidth || cv.width;
+        if (width) {
+            return width;
+        }
         const cname = cv.label || cv.name;
-        var width = Math.max(cname.length, get(cv, 'units.length', 0),  get(cv, 'type.length', 0));
+        width = Math.max(cname.length, get(cv, 'units.length', 0),  get(cv, 'type.length', 0));
         width = dataAry.reduce( (maxWidth, row) => {
             return Math.max(maxWidth, get(row, [idx, 'length'], 0));
         }, width);  // max width of data

src/firefly/js/tables/__tests__/FilterInfo-test.js

@@ -0,0 +1,47 @@
+
+import * as TblUtil from '../TableUtil.js';         // used for named import
+import {FilterInfo} from '../FilterInfo.js';
+
+describe('FilterInfo', () => {
+
+    test('autoCorrectConditions: string columns', () => {
+
+        /*
+        *  This test is a little trickier.   conditionValidator() takes a tbl_id that uses TblUtil.getTblById to resolve a tableModel.
+        *  This involves a fully functional redux store which is not available to our JS test environment at the moment.
+        *  So, we will 'fake' the return value of getTblById by 'mocking' that function
+        */
+        const aStringColumn = {
+            tableData: {
+                columns: [ {name: 'desc', type: 'char'}],
+            }
+        };
+
+        TblUtil.getTblById = jest.fn().mockReturnValue(aStringColumn);          // mock getTblById to return the aStringColumn table
+
+        let actual = FilterInfo.conditionValidator('=abc', 'a_fake_tbl_id', 'desc');
+        expect(actual.valid).toBe(true);
+        expect(actual.value).toBe("= 'abc'");      // the validator correctly insert space and quote around a value of a string column.
+
+        actual = FilterInfo.conditionValidator('abc', 'a_fake_tbl_id', 'desc');
+        expect(actual.valid).toBe(true);
+        expect(actual.value).toBe("like '%abc%'");      // the validator correctly convert it into a LIKE operator
+    });
+
+    test('autoCorrectConditions: numeric columns', () => {
+
+        const aNumericColumn = {
+            tableData: {
+                columns: [ {name: 'ra', type: 'double'}],
+            }
+        };
+
+        TblUtil.getTblById = jest.fn().mockReturnValue(aNumericColumn);          // once again mock getTblById to return the different (numeric) table
+
+        const {valid, value} = FilterInfo.conditionValidator('>1.23', 'a_fake_tbl_id', 'ra');
+        expect(valid).toBe(true);
+        expect(value).toBe('> 1.23');      // the validator correctly insert space and no quotes on numeric columns.
+    });
+
+});
+