deephaven / deephaven-core

Deephaven Community Core
Other
257 stars 80 forks source link

Failed to apply filters with string literal applied to Instant column #6315

Open mofojed opened 2 weeks ago

mofojed commented 2 weeks ago

Description

A clear and concise description of what the bug is.

Steps to reproduce

  1. Start up a server with Anonymous auth, then go to http://localhost:10000/jsapi/table_viewport.html and run the following in the browser console
    var dh = (await import('./dh-core.js')).default
    var client = new dh.CoreClient(window.location.protocol + "//" + window.location.host);
    await client.login({type:dh.CoreClient.LOGIN_TYPE_ANONYMOUS});
    connection = await client.getAsIdeConnection();
    ide = await connection.startSession("python");
    await ide.runCode(`
    from deephaven import time_table
    remoteTable = time_table("PT00:00:01").update_view(["I=i", "J=I*I", "K=I%100"]).last_by(by = ["K"])
    `)
    var t = await ide.getTable('remoteTable')
    var timeValue = '1730469472641000000';
    var timeColumn = t.findColumn('Timestamp')
    var timeFilter = timeColumn.filter().lessThan(dh.FilterValue.ofNumber(timeValue))
    t.applyFilter([timeFilter])

Expected results

  1. String value should be parsed as number, filter should apply

Actual results

  1. There is an error applying the filter:
    Failed to apply filters: [,,,,,Timestamp,,,1730469472641000000] Details Logged w/ID '6c5b5b0d-8834-4f32-8caf-ab9856ab3c22'

    Error details in the server logs:

    heduler-Concurrent-2 | i.d.s.s.SessionService    | Internal Error '6c5b5b0d-8834-4f32-8caf-ab9856ab3c22' java.lang.IllegalArgumentException: Failed to convert literal value <1730469472640999940> for column "Timestamp" of type java.time.Instant
    at io.deephaven.engine.table.impl.select.MatchFilter$ColumnTypeConvertor.convertValue(MatchFilter.java:383)
    at io.deephaven.engine.table.impl.select.RangeFilter.init(RangeFilter.java:185)
    at io.deephaven.engine.table.impl.QueryTable.initializeAndPrioritizeFilters(QueryTable.java:1185)
    at io.deephaven.engine.table.impl.QueryTable.lambda$whereInternal$30(QueryTable.java:1235)
    at io.deephaven.engine.table.impl.perf.QueryPerformanceRecorder.withNugget(QueryPerformanceRecorder.java:369)
    at io.deephaven.engine.table.impl.QueryTable.whereInternal(QueryTable.java:1233)
    at io.deephaven.engine.table.impl.QueryTable.where(QueryTable.java:1172)
    at io.deephaven.engine.table.impl.QueryTable.where(QueryTable.java:98)
    at io.deephaven.server.table.ops.FilterTableGrpcImpl.create(FilterTableGrpcImpl.java:57)
    at io.deephaven.server.table.ops.FilterTableGrpcImpl.create(FilterTableGrpcImpl.java:30)
    at io.deephaven.server.table.ops.TableServiceGrpcImpl$BatchExportBuilder.doExport(TableServiceGrpcImpl.java:757)
    at io.deephaven.server.session.SessionState$ExportObject.doExport(SessionState.java:995)
    at java.base/java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:515)
    at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
    at java.base/java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.run(ScheduledThreadPoolExecutor.java:304)
    at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128)
    at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628)
    at io.deephaven.server.runner.scheduler.SchedulerModule$ThreadFactory.lambda$newThread$0(SchedulerModule.java:100)
    at java.base/java.lang.Thread.run(Thread.java:829)
    Caused by: java.lang.IllegalArgumentException: Instant literal not enclosed in single-quotes ("1730469472640999940")
    at io.deephaven.engine.table.impl.select.MatchFilter$ColumnTypeConvertorFactory$13.convertStringLiteral(MatchFilter.java:721)
    at io.deephaven.engine.table.impl.select.MatchFilter$ColumnTypeConvertor.convertValue(MatchFilter.java:381)

Additional details and attachments

image

If applicable, add any additional screenshots, logs, or other attachments to help explain your problem.

Versions Engine Version: 0.36.1 Web UI Version: 0.90.0 Java Version: 11.0.24 Barrage Version: 0.6.0 Browser Name: Chrome 129 OS Name: Linux @deephaven/js-plugin-plotly-express: 0.11.2 @deephaven/js-plugin-ui: 0.22.0

rcaudy commented 2 weeks ago

I can't run the reproducer yet, but I notice that the literal does indeed seem to be missing single-quotes.

mofojed commented 2 weeks ago

This works on DHE, from the irisapi/table_viewport.html page in the browser console run:

var wcd = c.getKnownConfigs().find(q => q.name === 'WebClientData')
var workspaceData = await wcd.getTable('workspaceData')
var timeValue = '1730469472641000000';
var timeColumn = workspaceData.findColumn('LastModifiedTime')
var timeFilter = timeColumn.filter().lessThan(dh.FilterValue.ofNumber(timeValue))
workspaceData.applyFilter([timeFilter])
mofojed commented 2 weeks ago

I can't run the reproducer yet, but I notice that the literal does indeed seem to be missing single-quotes.

@rcaudy You need to start the server in Anonymous authentication mode for the snippet to work, I'm assuming you did not and that could be the source of your connection errors.

niloc132 commented 2 weeks ago

This is one of those "we got lucky" sorts of situations, and I'm going to investigate putting up a guardrail to make this failure more obvious, rather than let it seem like this is unexpectedly broken. Short version, comparing a date against a number means we're subject to JS's Number limitations, where an exact match of a timestamp will usually fail, and range matches will be subtly off.

FilterValue.ofNumber will accept a String, but can't do a lot with it - the best it can do is parse it to a number and assume that the numeric type selected is correct. The assumption exists that if the type matters and you want a long or instant, you'll use LongWrapper or DateWrapper, otherwise you're going to get a Js Number.

Given a value like "1254898894896696881", this will be truncated to 1254898894896696800, as JS Number is defined to be an IEEE 754 64bit float. If the row to match equals the first value, you may end up matching more/less than what is expected when the second value is sent to the server.

Instead, it is expected that FilterValue.ofNumber will be passed a dh.DateWrapper instance - perhaps the actual value read from the column to be filtered against, or created using the DateWrapper.ofJsDate method. We might need more factory methods here, to take strings or LongWrappers and convert to dates?