Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add callback functionality to Options/Context to capture visits to getLocalDecodedVector #87

Closed
wants to merge 2 commits into from

Conversation

kunalkataria
Copy link

Summary:
As title, in order to verify passthrough code is being correctly called and the local decoded vector is NOT being accessed, we need a way to track visits to the decode function. On suggestion/idea by helfman, we've created a visit callback passed in via the writer options which allows us to track if the decode visit has been called from the writer code.

I did verify that the callback is being called through some tests that I know do use the decode functionality. I plan to add decode visit counters for those tests later but that is out of scope for this diff for now

This decode visit functionality will be used in the flatmap passthrough functionality as well

Reviewed By: helfman

Differential Revision: D60241883

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Meta Open Source bot. label Sep 26, 2024
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D60241883

kunalkataria pushed a commit that referenced this pull request Oct 10, 2024
…tLocalDecodedVector (#87)

Summary:

As title, in order to verify passthrough code is being correctly called and the local decoded vector is NOT being accessed, we need a way to track visits to the decode function. On suggestion/idea by helfman, we've created a visit callback passed in via the writer options which allows us to track if the decode visit has been called from the writer code. 

I did verify that the callback is being called through some tests that I know _do_ use the decode functionality. I plan to add decode visit counters for those tests later but  that is out of scope for this diff for now

This decode visit functionality will be used in the flatmap passthrough functionality as well

Reviewed By: helfman

Differential Revision: D60241883
kunalkataria pushed a commit that referenced this pull request Oct 10, 2024
…tLocalDecodedVector (#87)

Summary:

As title, in order to verify passthrough code is being correctly called and the local decoded vector is NOT being accessed, we need a way to track visits to the decode function. On suggestion/idea by helfman, we've created a visit callback passed in via the writer options which allows us to track if the decode visit has been called from the writer code. 

I did verify that the callback is being called through some tests that I know _do_ use the decode functionality. I plan to add decode visit counters for those tests later but  that is out of scope for this diff for now

This decode visit functionality will be used in the flatmap passthrough functionality as well

Reviewed By: helfman

Differential Revision: D60241883
kunalkataria pushed a commit that referenced this pull request Oct 10, 2024
…tLocalDecodedVector (#87)

Summary:

As title, in order to verify passthrough code is being correctly called and the local decoded vector is NOT being accessed, we need a way to track visits to the decode function. On suggestion/idea by helfman, we've created a visit callback passed in via the writer options which allows us to track if the decode visit has been called from the writer code. 

I did verify that the callback is being called through some tests that I know _do_ use the decode functionality. I plan to add decode visit counters for those tests later but  that is out of scope for this diff for now

This decode visit functionality will be used in the flatmap passthrough functionality as well

Reviewed By: helfman

Differential Revision: D60241883
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D60241883

kunalkataria pushed a commit that referenced this pull request Oct 10, 2024
…tLocalDecodedVector (#87)

Summary:

As title, in order to verify passthrough code is being correctly called and the local decoded vector is NOT being accessed, we need a way to track visits to the decode function. On suggestion/idea by helfman, we've created a visit callback passed in via the writer options which allows us to track if the decode visit has been called from the writer code. 

I did verify that the callback is being called through some tests that I know _do_ use the decode functionality. I plan to add decode visit counters for those tests later but  that is out of scope for this diff for now

This decode visit functionality will be used in the flatmap passthrough functionality as well

Reviewed By: helfman

Differential Revision: D60241883
kunalkataria pushed a commit that referenced this pull request Oct 18, 2024
…tLocalDecodedVector (#87)

Summary:

As title, in order to verify passthrough code is being correctly called and the local decoded vector is NOT being accessed, we need a way to track visits to the decode function. On suggestion/idea by helfman, we've created a visit callback passed in via the writer options which allows us to track if the decode visit has been called from the writer code. 

I did verify that the callback is being called through some tests that I know _do_ use the decode functionality. I plan to add decode visit counters for those tests later but  that is out of scope for this diff for now

This decode visit functionality will be used in the flatmap passthrough functionality as well

Reviewed By: helfman

Differential Revision: D60241883
kunalkataria pushed a commit that referenced this pull request Oct 18, 2024
…tLocalDecodedVector (#87)

Summary:

As title, in order to verify passthrough code is being correctly called and the local decoded vector is NOT being accessed, we need a way to track visits to the decode function. On suggestion/idea by helfman, we've created a visit callback passed in via the writer options which allows us to track if the decode visit has been called from the writer code. 

I did verify that the callback is being called through some tests that I know _do_ use the decode functionality. I plan to add decode visit counters for those tests later but  that is out of scope for this diff for now

This decode visit functionality will be used in the flatmap passthrough functionality as well

Reviewed By: helfman

Differential Revision: D60241883
kunalkataria pushed a commit that referenced this pull request Oct 18, 2024
…tLocalDecodedVector (#87)

Summary:

As title, in order to verify passthrough code is being correctly called and the local decoded vector is NOT being accessed, we need a way to track visits to the decode function. On suggestion/idea by helfman, we've created a visit callback passed in via the writer options which allows us to track if the decode visit has been called from the writer code. 

I did verify that the callback is being called through some tests that I know _do_ use the decode functionality. I plan to add decode visit counters for those tests later but  that is out of scope for this diff for now

This decode visit functionality will be used in the flatmap passthrough functionality as well

Reviewed By: helfman

Differential Revision: D60241883
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D60241883

kunalkataria pushed a commit that referenced this pull request Oct 30, 2024
…tLocalDecodedVector (#87)

Summary:

As title, in order to verify passthrough code is being correctly called and the local decoded vector is NOT being accessed, we need a way to track visits to the decode function. On suggestion/idea by helfman, we've created a visit callback passed in via the writer options which allows us to track if the decode visit has been called from the writer code. 

I did verify that the callback is being called through some tests that I know _do_ use the decode functionality. I plan to add decode visit counters for those tests later but  that is out of scope for this diff for now

This decode visit functionality will be used in the flatmap passthrough functionality as well

Reviewed By: helfman

Differential Revision: D60241883
kunalkataria pushed a commit that referenced this pull request Oct 30, 2024
…tLocalDecodedVector (#87)

Summary:

As title, in order to verify passthrough code is being correctly called and the local decoded vector is NOT being accessed, we need a way to track visits to the decode function. On suggestion/idea by helfman, we've created a visit callback passed in via the writer options which allows us to track if the decode visit has been called from the writer code. 

I did verify that the callback is being called through some tests that I know _do_ use the decode functionality. I plan to add decode visit counters for those tests later but  that is out of scope for this diff for now

This decode visit functionality will be used in the flatmap passthrough functionality as well

Reviewed By: helfman

Differential Revision: D60241883
kunalkataria pushed a commit that referenced this pull request Oct 30, 2024
…tLocalDecodedVector (#87)

Summary:

As title, in order to verify passthrough code is being correctly called and the local decoded vector is NOT being accessed, we need a way to track visits to the decode function. On suggestion/idea by helfman, we've created a visit callback passed in via the writer options which allows us to track if the decode visit has been called from the writer code. 

I did verify that the callback is being called through some tests that I know _do_ use the decode functionality. I plan to add decode visit counters for those tests later but  that is out of scope for this diff for now

This decode visit functionality will be used in the flatmap passthrough functionality as well

Reviewed By: helfman

Differential Revision: D60241883
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D60241883

kunalkataria pushed a commit that referenced this pull request Oct 30, 2024
…tLocalDecodedVector (#87)

Summary:

As title, in order to verify passthrough code is being correctly called and the local decoded vector is NOT being accessed, we need a way to track visits to the decode function. On suggestion/idea by helfman, we've created a visit callback passed in via the writer options which allows us to track if the decode visit has been called from the writer code. 

I did verify that the callback is being called through some tests that I know _do_ use the decode functionality. I plan to add decode visit counters for those tests later but  that is out of scope for this diff for now

This decode visit functionality will be used in the flatmap passthrough functionality as well

Reviewed By: helfman

Differential Revision: D60241883
kunalkataria pushed a commit that referenced this pull request Nov 4, 2024
…tLocalDecodedVector (#87)

Summary:

As title, in order to verify passthrough code is being correctly called and the local decoded vector is NOT being accessed, we need a way to track visits to the decode function. On suggestion/idea by helfman, we've created a visit callback passed in via the writer options which allows us to track if the decode visit has been called from the writer code. 

I did verify that the callback is being called through some tests that I know _do_ use the decode functionality. I plan to add decode visit counters for those tests later but  that is out of scope for this diff for now

This decode visit functionality will be used in the flatmap passthrough functionality as well

Reviewed By: helfman

Differential Revision: D60241883
kunalkataria pushed a commit that referenced this pull request Nov 4, 2024
…tLocalDecodedVector (#87)

Summary:

As title, in order to verify passthrough code is being correctly called and the local decoded vector is NOT being accessed, we need a way to track visits to the decode function. On suggestion/idea by helfman, we've created a visit callback passed in via the writer options which allows us to track if the decode visit has been called from the writer code. 

I did verify that the callback is being called through some tests that I know _do_ use the decode functionality. I plan to add decode visit counters for those tests later but  that is out of scope for this diff for now

This decode visit functionality will be used in the flatmap passthrough functionality as well

Reviewed By: helfman

Differential Revision: D60241883
kunalkataria pushed a commit that referenced this pull request Nov 4, 2024
…tLocalDecodedVector (#87)

Summary:

As title, in order to verify passthrough code is being correctly called and the local decoded vector is NOT being accessed, we need a way to track visits to the decode function. On suggestion/idea by helfman, we've created a visit callback passed in via the writer options which allows us to track if the decode visit has been called from the writer code. 

I did verify that the callback is being called through some tests that I know _do_ use the decode functionality. I plan to add decode visit counters for those tests later but  that is out of scope for this diff for now

This decode visit functionality will be used in the flatmap passthrough functionality as well

Reviewed By: helfman

Differential Revision: D60241883
kunalkataria pushed a commit that referenced this pull request Nov 4, 2024
…tLocalDecodedVector (#87)

Summary:

As title, in order to verify passthrough code is being correctly called and the local decoded vector is NOT being accessed, we need a way to track visits to the decode function. On suggestion/idea by helfman, we've created a visit callback passed in via the writer options which allows us to track if the decode visit has been called from the writer code. 

I did verify that the callback is being called through some tests that I know _do_ use the decode functionality. I plan to add decode visit counters for those tests later but  that is out of scope for this diff for now

This decode visit functionality will be used in the flatmap passthrough functionality as well

Reviewed By: helfman

Differential Revision: D60241883
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D60241883

Kunal Kataria added 2 commits November 6, 2024 11:35
Summary:

Allows for already encoded dictionaries to passthrough the writer as opposed to being decoded + deduped again.

Specifically, we want to allow for a "passthrough" without decoding when the schema indicates that we are ingesting an ArrayVector, but the array we find is a dictionaryVector with ArrayVector values. When this is the case, we will check to see if the dictionary is a valid run-length encoding (meaning, it likely has been deduped by upstream). The way in which the vector is written to storage does not change, we will still write the offsets and the ArrayVector elements. 

A caveat is that right now, `isDictionaryValidRunLengthEncoded` function doesn't actually verify that there is a duplicate. It's possible that a scenario exists in which a DictionaryVector was passed to the ArrayVector field writer, but the DictionaryVector has not been deduped, but its offsets still technically fulfill a run-length encoding (each offset is greater than or equal to the offset at index - 1).

Reviewed By: helfman

Differential Revision: D59863671
…tLocalDecodedVector (#87)

Summary:

As title, in order to verify passthrough code is being correctly called and the local decoded vector is NOT being accessed, we need a way to track visits to the decode function. On suggestion/idea by helfman, we've created a visit callback passed in via the writer options which allows us to track if the decode visit has been called from the writer code. 

I did verify that the callback is being called through some tests that I know _do_ use the decode functionality. I plan to add decode visit counters for those tests later but  that is out of scope for this diff for now

This decode visit functionality will be used in the flatmap passthrough functionality as well

Reviewed By: helfman

Differential Revision: D60241883
kunalkataria pushed a commit that referenced this pull request Nov 6, 2024
…tLocalDecodedVector (#87)

Summary:

As title, in order to verify passthrough code is being correctly called and the local decoded vector is NOT being accessed, we need a way to track visits to the decode function. On suggestion/idea by helfman, we've created a visit callback passed in via the writer options which allows us to track if the decode visit has been called from the writer code. 

I did verify that the callback is being called through some tests that I know _do_ use the decode functionality. I plan to add decode visit counters for those tests later but  that is out of scope for this diff for now

This decode visit functionality will be used in the flatmap passthrough functionality as well

Reviewed By: helfman

Differential Revision: D60241883
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D60241883

kunalkataria pushed a commit that referenced this pull request Nov 6, 2024
…tLocalDecodedVector (#87)

Summary:

As title, in order to verify passthrough code is being correctly called and the local decoded vector is NOT being accessed, we need a way to track visits to the decode function. On suggestion/idea by helfman, we've created a visit callback passed in via the writer options which allows us to track if the decode visit has been called from the writer code. 

I did verify that the callback is being called through some tests that I know _do_ use the decode functionality. I plan to add decode visit counters for those tests later but  that is out of scope for this diff for now

This decode visit functionality will be used in the flatmap passthrough functionality as well

Reviewed By: helfman

Differential Revision: D60241883
kunalkataria pushed a commit that referenced this pull request Nov 6, 2024
…tLocalDecodedVector (#87)

Summary:

As title, in order to verify passthrough code is being correctly called and the local decoded vector is NOT being accessed, we need a way to track visits to the decode function. On suggestion/idea by helfman, we've created a visit callback passed in via the writer options which allows us to track if the decode visit has been called from the writer code. 

I did verify that the callback is being called through some tests that I know _do_ use the decode functionality. I plan to add decode visit counters for those tests later but  that is out of scope for this diff for now

This decode visit functionality will be used in the flatmap passthrough functionality as well

Reviewed By: helfman

Differential Revision: D60241883
@facebook-github-bot
Copy link
Contributor

This pull request has been merged in 3ab4c73.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Meta Open Source bot. fb-exported Merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants