Does replace_placeholders really work as intended?

[mpf82]

I assumed I could use this function to resolve binding parameters.

e.g.:

query = "SELECT * FROM tbl WHERE col1 = ? AND col2 = ?"
exp.replace_placeholders( parse_one(query), "abc", 123 )

Expected: "SELECT * FROM tbl WHERE col1 = 'abc' AND col2 = 123"

Actually: ValueError: Name needs to be a string or an Identifier, got: <class 'int'>

If I write

exp.replace_placeholders( parse_one(query), 'abc', '123' )

it results in

SELECT * FROM tbl WHERE col1 = abc AND col2 = "123" - which is invalid SQL because of the double quotes, also abc will be used as a column, not a literal.

Thus my questions:

• should replace_placeholders be reworked?
• should we add a new function "replace_binding_parameters", that does not try to cast everything to an identifier and instead uses strings as 'str' (by adding single quotes) and literally replaces numeric values in the statement?

[georgesittas]

It's been some time since we added this, so I might be forgetting something, but what you're saying feels reasonable to me. Maybe we need to have replace_placeholders do raw replacements instead of converting to identifiers.

@tobymao did we want to implement this only for identifiers for some reason? If not, happy to rework this, as Mike suggested.

[tobymao]

replace_placeholders should be calling convert so that passing in "x" -> Literal.string, if you want to replace a table or column, the user needs to pass in an identifier directly

[georgesittas]

Fixed in 2d243af