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

Fix issues due to mask aliasing #15680

Merged
merged 2 commits into from
Jan 17, 2023
Merged

Fix issues due to mask aliasing #15680

merged 2 commits into from
Jan 17, 2023

Conversation

martint
Copy link
Member

@martint martint commented Jan 12, 2023

Fix aliasing of fields in mask expressions

This causes two problems:

  • Masks can inadvertently refer to columns that appear earlier
    in the list of columns as the projection is planned. This
    causes the mask expression to see the masked value of other columns
    instead of the underlying value
  • A possible bug in other optimizers causes mask expressions to be
    lost when the result of a such an expression is of type ROW
    and there's a dereference of a field downstream

Fixes #15659

Relates to #14420

Release notes

(x) Release notes are required, with the following suggested text:

# General
* Fix incorrect results when referencing a field resulting from the application of a column mask expression that produces a `row` type. ({issue}`15659`)
* Fix incorrect application of column masks when a mask expression references a different column in the underlying table. ({issue}`15680`)

# SPI
* Remove support for multiple masks on a single column. ({issue}`15680`)

Copy link
Member

@kokosing kokosing left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM % comments

@martint martint force-pushed the mask-aliasing branch 4 times, most recently from 042eb62 to e29729b Compare January 14, 2023 00:05
@martint martint requested a review from kokosing January 14, 2023 03:19
@martint martint force-pushed the mask-aliasing branch 2 times, most recently from 9f80643 to 5be85e4 Compare January 17, 2023 01:47
This is problematic because:
* There is no guarantee about the ordering of application of masks,
  which could result in non-deterministic results at best or
  query failures in the worst case
* Allowing multiple masks, especially when provided by different
  connectors, means that one cannot reason about a mask expression
  in isolation with respect to the input the expression expects.
This causes two problems:
* Masks can inadvertently refer to columns that appear earlier
  in the list of columns as the projection is planned. This
  causes the mask expression to see the masked value of other columns
  instead of the underlying value
* A possible bug in other optimizers causes mask expressions to be
  lost when the result of a such an expression is of type ROW
  and there's a dereference of a field downstream
@martint martint merged commit 9414f43 into trinodb:master Jan 17, 2023
@github-actions github-actions bot added this to the 406 milestone Jan 17, 2023
@martint martint deleted the mask-aliasing branch January 17, 2023 22:22
utk-spartan added a commit to utk-spartan/ranger that referenced this pull request Mar 8, 2023
Incorporate following changes

Remove deprecated checkCanCreateSchema
trinodb/trino#15618
trinodb/trino@0fac087

Disallow multiple masks on a given column
trinodb/trino#15680
trinodb/trino@bdd1cb5
utk-spartan added a commit to razorpay/ranger that referenced this pull request Mar 8, 2023
Incorporate following changes

Remove deprecated checkCanCreateSchema
trinodb/trino#15618
trinodb/trino@0fac087

Disallow multiple masks on a given column
trinodb/trino#15680
trinodb/trino@bdd1cb5
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

Security issue with access control: column masking gets lost when querying only nested field
4 participants