* fix: column summary conversion for "unknown" TS
Both IOx and DataFusion have the same data model for min/max statistics:
`Option<Option<i64>>` (or any other inner type)
The interpretation is:
1. **`None`:** Value unknown.
2. **`Some(None)`:** Value known to be NULL.
3. **`Some(Some(x))`:** Value known and non NULL.
The bug was that during the conversion from the IOx statistics type to
the DataFusion statistics type for timestamps, case 1 was converted into
case 2.
Up until now this didn't make a difference between timestamps were
basically known all the time, but during the development of NG there are
cases where the timestamps are unknown (this might change, but the query
engine should be correct w/o assuming that).
* docs: explain test
Co-authored-by: Andrew Lamb <andrew@nerdnetworks.org>
Co-authored-by: Andrew Lamb <andrew@nerdnetworks.org>
Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
* test: Failing test for finding overlapped groups
* test: Failing test for query overlap too :(
* fix: Group parquet files overlapped by time correctly
Inspired by https://towardsdatascience.com/overlapping-time-period-problem-b7f1719347db
Not sure what the real name for this algorithm is
* refactor: Group items without an intermediate hashmap needed
* chore: cleanup
Co-authored-by: NGA-TRAN <nga-tran@live.com>
Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
* test: use Paul deadlock reproducer and add more debug log
* test: remove compare many output rows
* test: verify the test putput
* chore: cleanup
Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
This commit resolves the compaction deadlock described in #4306.
The deadlock occurs during StreamSplitExec execution, where a background
worker is spawned to read input record batches and partition them into
two groups. This code pushes the resulting split record batches into two
channels - one for records that match a given predicate, and another
channel for those that do not. These channels buffer at most 2 record
batches each.
The compactor that executes this plan reads the resulting partitions
sequentially to completion. Completion is indicated by reading until the
results stream ends, which ends when the underlying channel is closed,
and therefore the split worker task must have finished and closed the
results channel for the partition to be successfully read.
While the compactor is reading from the first partition, the worker is
attempting to push record batches into the second partition and blocks
due to the channel capacity being reached. The worker never drops the
channel for the first partition, so the compactor never finishes reading
the first partition, and nothing is reading the second partition to
unblock the worker. Deadlock!
* chore: add more compactor debug info
* chore: Apply suggestions from code review
Co-authored-by: Andrew Lamb <andrew@nerdnetworks.org>
* chore: fix format
Co-authored-by: Andrew Lamb <andrew@nerdnetworks.org>
Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
* fix: not to add IOxReadFilterNode for no data of non-duplicated chunks if there is already scan node for overlapped/duplicated chunks
* refactor: address review comments
* chore: Apply suggestions from code review
Co-authored-by: Carol (Nichols || Goulding) <193874+carols10cents@users.noreply.github.com>
Co-authored-by: Carol (Nichols || Goulding) <193874+carols10cents@users.noreply.github.com>
Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
This allows us to remove the table name from the low-level chunk
representations (like `ParquetFile`, RUB, ...) since table names are
already tracked by the higher-level data structures (e.g. catalog,
catalog chunk) that manage the low-level chunk representations.
This is similar to #4167.
Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
Min/max values and distinct counts are already optional, so let's make
the null counts optional as well. This will be helpful for NG to deal w/
partial statistics (e.g. we only populate stats for the time column).
Note that the total count is still mandatory, but we normally have the
chunk/file-level row count at hand.
* refactor: dyn-dispatch database in query subsystem
This is similar to #4080 but concerns the database itself.
For #3934.
* docs: improve wording
Co-authored-by: Andrew Lamb <andrew@nerdnetworks.org>
Co-authored-by: Andrew Lamb <andrew@nerdnetworks.org>
Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
- this is what DataFusion is doing as well; it's also fast enough
because the number of chunks in a query is not THAT massive (it's not
like we are doing row-level dyn dispatching)
- it simplifies abstracting over different databases
- it allows us to drop our enum-based dispatching that we have for
`DbChunk` and that we would also need for the querier (e.g. depending
on if a chunk is backed by a parquet file or ingester data)
- it likely speeds up compile times because the `query` is no longer
contains massive amounts of generic code
For #3934.
This makes it way easier to dyn-type database implementations. The only
real change is that we make `QueryChunk::Error` opaque. Nobody is going
to inspect that anyways, it's just printed to the user.
This is a follow-up of #4053.
Ref #3934.
For OG we can determine the chunks w/o any IO, for NG however this might
require a few catalog queries.
This is likely not the last change of this sort, i.e. the whole schema
handling is currently sync as well.
Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
* feat: initial implementation of compact a given list of overlapped parquet files
* feat: Add QueryableParquetChunk and some refactoring
* feat: build queryable parquet chunks for parquet files with tombstones
* feat: second half the implementation for Compactor's compact. Tests will be next
* fix: comments for trait funnctions fof QueryChunkMeta
* test: add tests for compactor's compact function
* fix: typos
* refactor: address Jake's review comments
* refactor: address Andrew's comments and add one more test for files in different order in the vector
Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
- This is not used by the query engine at all.
- The query engine should not care about ALL chunks but only about the
chunks it gets via `QueryDatabase::chunks` (which includes a table
name and a predicate).
- All other users of that API are NOT really query-related.
- This was not actually used by the query engine.
- The query engine doesn't have a concept of a "partition", it only
cares about chunks.
- Unbound access to all partitions in the database is quite expensive
(esp. on NG).
* refactor: wire exectution context to Deduplicator
* feat: example trace to chunk read_filter
* refactor: make execution context required
* refactor: expose metadata API
* refactor: more span context for chunk read_filter
* refactor: fix build
* refactor: push context into result stream
* refactor: make executor optional
Marco Neumann
Andrew Lamb
Carol (Nichols || Goulding)
Nga Tran
Dom Dwyer
kodiakhq[bot]
Carol (Nichols || Goulding)
dependabot[bot]
Raphael Taylor-Davies
Edd Robinson