Protocol question: multiple logical files pointing to the same data file path

[shanisolomon]

I have a delta table with 2 versions:

Add txn: path = "a.parquet" numRecords = 10 deletionVector = null

Add txn: path = "a.parquet" numRecords = 10 deletionVector = (..., cardinality = 2)

Please note both transactions point to the same physical path ("a.parquet"), without any remove transaction.

From my understanding of the delta protocol, since the above are 2 separate logical files residing in two different versions, the above describes a legal delta table that when queried, should return 18 rows.

Could you please confirm my understanding?

[felipepessoto]

I tough the same, but protocol forbids it:

“it is illegal for the same path to be occur twice with different dvIds within each set of add or remove actions.”

[shanisolomon]

@felipepessoto I'm afraid I think your quote speaks of a specific commit (txn), and not the entire log. This is the full paragraph:

`Since actions within a given Delta commit are not guaranteed to be applied in order, a valid version is restricted to contain at most one file action of the same type (i.e. add/remove) for any one combination of path and dvId. Moreover, for simplicity it is required that there is at most one file action of the same type for any path (regardless of dvId). That means specifically that for any commit…

it is legal for the same path to occur in an add action and a remove action, but with two different dvIds.
it is legal for the same path to be added and/or removed and also occur in a cdc action.
it is illegal for the same path to be occur twice with different dvIds within each set of add or remove actions.`

From the above I read:

1. In a delta log, different (path, dvId) combination can exists at most once for each add or remove actions. In the example I provided above, that holds.
2. Within a single commit, only one set of a specific (path, dvId) combination can exist for each add or remove actions. In the example I provided above, that also holds.

[shanisolomon]

Adding @larsk-db who added the protocol changes:
16dad5a

[larsk-db]

Yes, this is legal, though likely not desirable except in cases where the DVs are different+non-overlapping.

The reason there are constraints like this across commits, is so that metadata-only updates are possible. So it's actually even legal to have:

• Commit 1: Add: path = "a.parquet" numRecords = 10 deletionVector = (dvId1, cardinality = 5)
• Commit 2: Add: path = "a.parquet" numRecords = 10 deletionVector = (dvId1, cardinality = 5)

However, the total here is not 10, it's 5, because those are the same files, so snapshot replay should pick the latter one (from a data perspective it doesn't even matter which one is picked, they point to the same data...but they could have different metadata, so query execution might end up looking different depending on which one is included in a snapshot).

[shanisolomon]

@larsk-db could you please further clarify your answer?

In the following example where we have 1 physical file, and 2 logical files:
Commit 1: Add: path = "a.parquet" numRecords = 10 deletionVector = (dvId1, cardinality = 5) // removed rows: 0,1,2,3,4
Commit 2: Add: path = "a.parquet" numRecords = 10 deletionVector = (dvId2, cardinality = 4) // removed rows: 0, 7,8,9

Q1: What rows are expected to be returned, when querying the above table?

Q2: Does creating a checkpoint changes the above result?

[larsk-db]

What rows are expected to be returned, when querying the above table?

Rows 5-9 and 1-6. Duplicating 5 and 6.

Does creating a checkpoint changes the above result?

No. There is no ambiguity in checkpoint creation here. These are different entries, as they have two different DVs.

[shanisolomon]

Thanks @larsk-db for the confirmation.
I'd like to also confirm the (perhaps obvious at this point) question, that the following delta table will result in rows 1 - 6 only:

Commit 1: Add: path = "a.parquet" numRecords = 10 deletionVector = (dvId1, cardinality = 5) // removed rows: 0,1,2,3,4
Commit 2: Add: path = "a.parquet" numRecords = 10 deletionVector = (dvId2, cardinality = 4) // removed rows: 0,7,8,9
Commit 3: Remove: path = "a.parquet" numRecords = 10 deletionVector = (dvId1, cardinality = 5)

[larsk-db]

Indeed.

[shanisolomon]

Thanks @larsk-db for the confirmation.

[zeevm]

@larsk-db
So Delta allows reusing the same physical file twice, but I have to delete some rows to do that?
If the file is to be used twice but without deleting rows (so no DV) then it's not supported?
That seems weirdly arbitrary.

[larsk-db]

What exactly are you doing to account for this? I'm pretty sure we do nothing. If someone writes weird commits, they get weird data. We don't write these kind of commits (at least not on purpose^^).

I mean, sure, we could forbid it in the protocol. But how many connectors would actually be able to enforce cross-commit requirements? That means (potentially) comparing a commit of thousands of entries with a snapshot of millions of entries by key. Probably doesn't fit into memory, so would have to be done distributed. Very expensive for just a sanity check.
So if no one implements the check, then the statement in the protocol is mostly aspirational (and in the worst case, misleading readers into a false sense of security, though as mentioned above, I don't think this particular weirdness is something that actually needs accounting for on the read-side).
There are already restrictions in there that I wasn't in favour of adding, because they are pretty much just aspirational because too expensive to check. Like the entire "It is legal/illegal" paragraph in the thread above can already be infeasible to check on really large commits. But at least large commits are not as common as large snapshots.

That being said, I probably wouldn't block the addition of an additional restriction. So if you want to augment the spec in a PR and get some wider reviews on that, please feel free.

[zeevm]

What exactly are you doing to account for this? I'm pretty sure we do nothing. If someone writes weird commits, they get weird data. We don't write these kind of commits (at least not on purpose^^).

That's true, trying to read such a transaction with Databricks did yield odd results, it returned all the rows (including the duplicates) but when doing a row count it only "sees" the physical file once, even though it appears in two logical files, would you consider this a Databricks bug?

[larsk-db]

Hmm, could be wrong handling of files with DVs somewhere in the metadata-only aggregation code, yeah. They should be producing consistently weird results, not differently weird^^

[zeevm]

"consistently weird results" is beautiful, I just may plagiarize it.

I'd think the protocol should be explicit about it, something like:

"In any given snapshot, it is illegal for the same path to occur more than once in "Add" actions."

WDYT, should I open a PR for this?

[felipepessoto]

When I tested this months ago my impression was that OSS also doesn’t support multiple addfiles with same path and different DV. That’s why I was asking about it too. If I remember correctly query works, but update/delete doesn’t.

Also agree protocol should forbid same path used multiple times even with different DV.
It could lead to weird bugs for example if two addfiles uses same path and different DV, and after a delete they both have same deleted row index, the Delta client could try to use the same DV file, but then we end up with two addfiles with same path and DV. (If using inline DV, it also could happen if same row indexes are deleted, and there is no workaround except not using inline DV)