Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
mofojed opened this issue Nov 1, 2024 · 4 comments
Open
Assignees
Labels
bug Something isn't working jsapi
Milestone

Comments

@mofojed
Copy link
Member

mofojed commented Nov 1, 2024

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

@mofojed mofojed added bug Something isn't working triage labels Nov 1, 2024
@mofojed mofojed changed the title Failed to apply filters Failed to apply filters with string literal applied to Instant column Nov 1, 2024
@rcaudy
Copy link
Member

rcaudy commented Nov 1, 2024

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

@mofojed
Copy link
Member Author

mofojed commented Nov 1, 2024

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
Copy link
Member Author

mofojed commented Nov 1, 2024

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.

@rcaudy rcaudy added jsapi and removed triage labels Nov 1, 2024
@rcaudy rcaudy added this to the 0.37.0 milestone Nov 1, 2024
@niloc132
Copy link
Member

niloc132 commented Nov 1, 2024

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working jsapi
Projects
None yet
Development

No branches or pull requests

3 participants