search

deephaven / deephaven-core

Deephaven Community Core
Other
257 stars 80 forks source link

Iceberg reading with explicit Schema support #6124

Open devinrsmith opened 1 month ago

devinrsmith commented 1 month ago

I believe we need to offer Iceberg reading support based on a user-specified Schema (likely a Schema that was sourced from some history in the Table, or some subset); in the context where a user is passing in String column renames, the keys of that map are tied to a specific Schema (in this way also, it can internally be converted into a field-id rename, which can be used across column renaming). It may also be important in an enterprise for them to establish and record the specific Schema a table was initially ingested as (for example, they may want to enforce that db.t("MyNamespace", "MyTable") produces the same exact output today as it did yesterday). It is not enough to simply record the latest, or even a specific, Snapshot because the schema of the Table may change without a new Snapshot - for example, when a column is renamed, the Table's Schema will be updated, but no new Snapshots will be created.

As a point of convention, it probably makes sense to assume these defaults (in order):

  1. If a schema is provided, use that schema
  2. If a snapshot is provided, use schema table.schemas().get(snapshot.schemaId())
  3. Otherwise, use schema table.schema(); note, this is not equivalent to table.schemas().get(table.currentSnapshot().schemaId()) for reasons mentioned above.
devinrsmith commented 1 month ago

I can also make an argument that our default should be:

  1. If a schema is provided, use that schema
  2. Otherwise, use schema table.schema()

IE, if providing a specific snapshot (and not a schema), the results would be the snapshot's data projected into the latest Table's schema.

This has the advantage that tableAdapter.read() would semantically be the same as tableAdapter.read(table.currentSnapshot()).

Regardless, in either regime, the user can specify the schema, and so achieve the behavior they desire.

devinrsmith commented 1 month ago

The table.schema() regime by default also allows the current constructors from io.deephaven.iceberg.layout.IcebergBaseLayout (and derived) to still be valid; while we should be okay breaking these, we eventually need to get into state where we aren't breaking them.

devinrsmith commented 1 month ago

Q: does PartitionSpec come into play, and if yes, is it always the case that the schema is equal to PartitionSpec#schema?

malhotrashivam commented 1 month ago

The same should also be applicable on iceberg writing side. Discussion here: https://github.com/deephaven/deephaven-core/pull/5989#discussion_r1791065313, https://github.com/deephaven/deephaven-core/pull/5989#discussion_r1791069848, and https://github.com/deephaven/deephaven-core/pull/5989#discussion_r1791071072