deephaven / deephaven-core

Deephaven Community Core
Other
254 stars 80 forks source link

Downsampling throws an error #1282

Open mofojed opened 3 years ago

mofojed commented 3 years ago

Description

When trying to display a plot with Downsampling, getting an error

Steps to reproduce

  1. Run the following code snippet (requires Deephaven examples to be set up in your data container):
    
    from deephaven.ParquetTools import readTable
    from deephaven.Plot import plot

taxiTable = readTable("/data/examples/taxi/parquet/taxi.parquet") p = plot("Plot", taxiTable, "tpep_pickup_datetime", "trip_distance").show()


**Expected results**

1. A table and a chart appear

**Actual results**

1. The table appears, the chart panel appears but is blank. Error is output to the logs

**Additional details and attachments**
![image](https://user-images.githubusercontent.com/4505624/133291546-6fb2a857-5729-438c-9c24-d82145ffeb1b.png)

heduler-Concurrent-3 | i.d.g.s.SessionState | Internal Error 'b118e384-0ffc-48d6-a515-ac47456ed326' java.lang.IllegalArgumentException: Can't downsample table of type class io.deephaven.db.v2.SimpleSourceTable at io.deephaven.clientsupport.plotdownsampling.RunChartDownsample.call(RunChartDownsample.java:129) at io.deephaven.clientsupport.plotdownsampling.RunChartDownsample.call(RunChartDownsample.java:49) at io.deephaven.db.tables.Table.apply(Table.java:2468) at io.deephaven.grpc_api.table.ops.RunChartDownsampleGrpcImpl.create(RunChartDownsampleGrpcImpl.java:35) at io.deephaven.grpc_api.table.ops.RunChartDownsampleGrpcImpl.create(RunChartDownsampleGrpcImpl.java:14) at io.deephaven.grpc_api.table.TableServiceGrpcImpl.lambda$null$14(TableServiceGrpcImpl.java:417) at io.deephaven.grpc_api.session.SessionState$ExportObject.doExport(SessionState.java:825) at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511) at java.util.concurrent.FutureTask.run(FutureTask.java:266) at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.access$201(ScheduledThreadPoolExecutor.java:180) at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.run(ScheduledThreadPoolExecutor.java:293) at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149) at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624) at io.deephaven.grpc_api.runner.DeephavenApiServerModule$ThreadFactory.lambda$newThread$0(DeephavenApiServerModule.java:147) at java.lang.Thread.run(Thread.java:748)

niloc132 commented 3 years ago

This never came up in DHE, I think we assumed it was always coalesced before it got to this point.

RunChartDownsample expects a QueryTable so that it can memoize results, which it uses to both share the downsample between connected users and also to avoid re-downsampling too rapidly if the user is resizing (and client is re-requesting the downsample).

All other operations it does can stick with a BaseTable, but a SimpleSourceTable (and other UncoalescedTables) is not a QueryTable, so doesn't memoize results.

@rcaudy @jcferretti any thoughts here? Should we change the error to encourage a .where() or .coalesce() on the console? I think in DHE we encourage PQ scripts to always coalesce in some way before exposing, perhaps that's how we handle app mode scripts?

rcaudy commented 3 years ago

coalesce() is a no-op or memoized. If you require coalesced input, call .coalesce() on it. Then you should be able to safely cast to QueryTable.