Use the execution-time user mapping for plan-time remote access - #331
Open
mattico wants to merge 2 commits into
Open
Use the execution-time user mapping for plan-time remote access#331mattico wants to merge 2 commits into
mattico wants to merge 2 commits into
Conversation
Plan-time remote accesses (the sql_mode/row-estimate connection in mysqlGetForeignRelSize, the cost-estimate option fetch in mysqlEstimateCosts, and the rowid-uniqueness probe in mysqlPlanForeignModify) resolved their user mapping (and the credentials mysql_get_options() reads from it) as the current user, while execution uses the RTE's checkAsUser when set (a view owner or SECURITY DEFINER context). A query through a view owned by another role therefore connected as (or failed to find a mapping for) the wrong user at plan time, failing with "user mapping not found" even though execution would have succeeded. Add mysql_plan_userid() and thread the effective user through every plan-time probe, and resolve it in mysqlBeginForeignModify() too (mtstate->ps.plan is the ModifyTable node, not a ForeignScan, so on PG 16+ the permission info must come from the result relation via ExecGetResultRelCheckAsUser()). mysql_plan_userid() reads the planner's RelOptInfo.userid (like postgres_fdw's baserel->userid) rather than the RTE: a foreign table that is a partition/inheritance child has no checkAsUser on its own RTE on PG 16+ (perminfoindex 0); the planner propagates the parent's effective user onto the child rel. Falling back to the RTE happens only when no RelOptInfo was built. On PG < 16 RelOptInfo.userid is itself set from rte->checkAsUser, so this is behavior-neutral there. mysql_get_options() takes the resolved userid; ANALYZE passes the table owner, IMPORT SCHEMA / TRUNCATE the current user. Covered by new view-owner and foreign-partition-through-view cases in connection_validation.
The param_values text array and its companion param_flinfo output- conversion functions were copied verbatim from postgres_fdw in commit 13b95af, where the parameters are shipped to the remote as text via libpq. mysql_fdw instead binds parameters as native MYSQL_BIND structs through mysql_bind_sql_var(), so the text form was never consumed: param_values was written by process_query_params() and never read, and param_flinfo existed solely to feed the discarded OutputFunctionCall(). Drop both struct fields, the per-parameter output-function lookup in prepare_query_params(), and the per-tuple OutputFunctionCall() in process_query_params(). param_types and the MYSQL_BIND binding path are unchanged, so behavior is identical while saving wasted work per row.
|
Thank you @mattico for submitting the pull request. I had quick look on this and it appears that we need userid handling in planning phase. The most of the code changes looks good to me. I will spend some more time on the review and testing and check if we can commit this. |
Author
|
Thank you @surajkharage19, let me know if I can answer any questions. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Plan-time remote accesses (the sql_mode/row-estimate connection in mysqlGetForeignRelSize, the cost-estimate option fetch in mysqlEstimateCosts, and the rowid-uniqueness probe in mysqlPlanForeignModify) resolved their user mapping, and the credentials mysql_get_options() reads from it, as the current user. Execution, however, uses the RTE's checkAsUser when set (a view owner or SECURITY DEFINER context). A query through a view owned by another role therefore connected as (or failed to find a mapping for) the wrong user at plan time:
Fix
Add mysql_plan_userid() and thread the effective user through every plan-time probe, and resolve it in mysqlBeginForeignModify() too (mtstate->ps.plan is the ModifyTable node, not a ForeignScan, so on PG 16+ the permission info must come from the result relation via ExecGetResultRelCheckAsUser()).
mysql_plan_userid() reads the planner's RelOptInfo.userid (like postgres_fdw's baserel->userid) rather than the RTE: a foreign table that is a partition/inheritance child has no checkAsUser on its own RTE on PG 16+ (perminfoindex 0); the planner propagates the parent's effective user onto the child rel. It falls back to the RTE only when no RelOptInfo was built. On PG < 16 RelOptInfo.userid is itself set from rte->checkAsUser, so this is behavior-neutral there.
mysql_get_options() takes the resolved userid; ANALYZE passes the table owner, IMPORT SCHEMA / TRUNCATE the current user.
A second commit removes the unused text-format parameter machinery noticed while working in this area: param_values / param_flinfo were copied from postgres_fdw (where parameters ship as text via libpq), but mysql_fdw binds native MYSQL_BIND structs, so the text form was computed per row and never read. No behavior change.
Testing
New view-owner and foreign-partition-through-view cases in the connection_validation suite. Includes
expected/connection_validation_1.outfor PostgreSQL 18, which reworded the error touser mapping not found for user "X", server "Y". Compiled warning-free against PostgreSQL 14 through 18; suite run against MariaDB 10.5-11.4 and MySQL 5.7/8.0/8.4 on PG 14 and 18. Test harness here: mattico@ee838e6