facebook / squangle

SQuangLe is a C++ API for accessing MySQL servers
Other
123 stars 54 forks source link

[ RFC ] Formatting qualified names with %N (like %C, but allow intermediate dots) #14

Open lexidor opened 3 years ago

lexidor commented 3 years ago

This RFC depends on hhvm's AsyncMysqlConnection->queryAsync() method.

The function appendColumnTableName() in mysql_client/Query.cpp does a runtime check on the QueryArgument passed. If it is a string, it places backticks around it and escapes any actual backticks by tossing in an extra backtick. If a twoTuple is passed, each element of the tuple is escaped this way and they are concatenated using a .. The threeTuple variant interprets the third element as an alias. Hhvm and Hack have a concept of a tuple(string, string). These tuple types do not match twoTuple and threeTuple. This makes this logic unreachable from hhvm. This means that %C can never be transformed into

`tablename`.`columnname` or `tablename`.`columnname` AS `alias`

Passing the string "t.c"` to %C renders as

`t.c` instead of `t`.`c`

This means you can not pass a qualified name, since the database parser trips over qualified names that have the dot in the backticked context. I do not remiss the aliased threeTuple, but not having a way to passing a qualified name hurts. Hack code can use %T.%C and pass two string arguments. Code that intends to operate with both qualified and unqualified names has to fall back to using %Q.

I would like to be able to use a formatting specifier with works with both kinds of names. This could be performed by tossing in a backtick before and after a . in a string. Adding this to %C will transform a query which fails to parse into one that parses. This may not be what we want if callers expected untrusted arguments to not be able to reach ambiguous fields (join queries with overlapping columns). It also breaks native CPP consumers which pass tuples. Can we introduce a new specifier (I was thinking %N for name) which only operates on strings and tosses in backticks around dots? We might want to throw if the string has more than two parts to prevent injections of database level qualified names (dbname.tablename.columnname).

fredemmott commented 3 years ago

Hhvm and Hack have a concept of a tuple(string, string). These tuple types do not match twoTuple and threeTuple. This makes this logic unreachable from hhvm.

It could be done as far as runtime is concerned by special-casing 2/3-element arrays in squangle, but this isn't possible in a way that's compatible with Hack's type system, so adding a new placeholder as suggested is the only way to do this without making hack code less safe.