feat(java): support statistics row num for lance scan #3304
Conversation
| try (LockManager.ReadLock readLock = lockManager.acquireReadLock()) { | ||
| Preconditions.checkArgument(nativeDatasetHandle != 0, "Dataset is closed"); | ||
| return nativeCountRows(); | ||
| } | ||
| } | ||
|
|
||
| private native int nativeCountRows(); | ||
| private native long nativeCountRows(); |
There was a problem hiding this comment.
we do support countRows with filter.
Should we implement this as nativeCountRows(Optional<String> filter) ?
There was a problem hiding this comment.
It's a good suggestion. We can add this method for the aggregator pushdown for spark connector.
In this pr, the spark statistics only need the total row number.
This statistic will be used in the optimizer. The nativeCountRows(Optional<String> filter) will use the cpu to do the filter, and it's not suitable for the statistics.
There was a problem hiding this comment.
I was thinking this as a Dataset public API aspect. we do need to expose the Dataset.java api as equivalent to the Rust Dataset API.
In spark, it is fine to just call nativeCountRows(None) for planning.
There was a problem hiding this comment.
Should add this count_filter in this pr?
There was a problem hiding this comment.
Can we have one native method and two override methods?
private native long nativeCountRows(/* Nullable */ String filter);
// Override
public long countRows();
public long countRows(String filter);
There was a problem hiding this comment.
Fine， I will add it later
| // TODO: Support quickly get the bytes on disk for the lance dataset | ||
| // Now use schema to infer the byte size for simple |
There was a problem hiding this comment.
I've proposed per-field storage stats here: #3328
I started and then backed away from a whole-dataset number because I ran into some questions.
No need to answer these now but something to think about.
Should a whole-dataset number include indices and manifests? Or just data? Or one number for just data and one number for everything?
Should it include files that have been removed from the current version but not yet cleaned up?
Should it include columns that have been dropped (these columns may still have data in old fragments)
There was a problem hiding this comment.
Hi @westonpace The more precise the data size is, the better the optimizer will work.
In other words, the Spark optimizer needs to trade off the precision of plan and the computational performance of each rules.
There was a problem hiding this comment.
The mostly usage of dataset storage size in spark was to choose use broadcast join or not. So if the accurate data size of lance dataset included the dropped column and deleted row only need O(N) io call (N for the number of fragment), I think we can use this api. If not, use the total size of all the data fragments is also fine for this case.
Support statistics row num for lance scan, and with this statistics the spark will choose the broadcast to join for a small table.
But now the byte size of lance dataset is inferred by row number. It is not very precise.
Maybe We should store the file size in meta as disscus in #3221.