Question: why %(var)s? Will it always be s?

Oh good question. I picked that final s because it's consistent with the only other e2e test we have that tests this behaviour. But since inject_parameters is just a convenience wrapper around python's string interpolation, it's very possible we have users with invocations like:

query = "SELECT * FROM users WHERE user_id = %(user_id)d"
parameters = {"user_id": 1234}
cursor.execute(query, parameters)

I can add an e2e test for this and revise the regex to capture-and-replace any valid format specifier. I wonder just how complex could be the format specifiers customers are using...depending on the edge-cases it could make sense to use a different approach for this.

What I'm thinking is that rather than regexing for the parameter marker, we could simply render it inline and then do str.replace() where we replace the rendered value with the new parameter marker. Thoughts?

This is the complete format Mini-Language specification.. For simplicity, I like the approach of simply rendering the pyformat marker with its parameter value and then replacing that with our new marker.

Oh good question. I picked that final s because it's consistent with the only other e2e test we have that tests this behaviour. But since inject_parameters is just a convenience wrapper around python's string interpolation, it's very possible we have users with invocations like:

query = "SELECT * FROM users WHERE user_id = %(user_id)d"
parameters = {"user_id": 1234}
cursor.execute(query, parameters)

I can add an e2e test for this and revise the regex to capture-and-replace any valid format specifier. I wonder just how complex could be the format specifiers customers are using...depending on the edge-cases it could make sense to use a different approach for this.

What I'm thinking is that rather than regexing for the parameter marker, we could simply render it inline and then do str.replace() where we replace the rendered value with the new parameter marker. Thoughts?

So, the only examples I could find all use %()s. Even for things like dates. Now technically, because you were just doing python string interpolation before, people might have used crazier things than that, but I don't think we have any obligation to support things outside of the norm. If they are doing crazy things, the rendered value could be unexpected (consider if they passed you date formatting for example). I think the simple regex approach is fine.

after this change if user use :param style query, it will work right?

@jadewang-db

after this change if user use :param style query, it will work right?

Yes, but only when the native parameter approach is used. It will fail at DBR when use_inline_params=True because DBR will see a parameter marker and expect that a TSparkParameter was passed in the request. But pysql will not have sent a TSparkParameter because the inline approach doesn't do that.

@benc-db

So, the only examples I could find all use %()s. Even for things like dates. Now technically, because you were just doing python string interpolation before, people might have used crazier things than that, but I don't think we have any obligation to support things outside of the norm. If they are doing crazy things, the rendered value could be unexpected (consider if they passed you date formatting for example). I think the simple regex approach is fine.

Works for me!

This is the complete format Mini-Language specification.. For simplicity, I like the approach of simply rendering the pyformat marker with its parameter value and then replacing that with our new marker.

I'm skeptical that this is possible without having exactly as complicated a regex. Consider that a date format could lead to the word August being rendered. In order to know that August is what you're looking for, you would need to extract their formatting code. But if you have a regex that can extract their formatting code, you may as well just use that for finding and replacing without rendering the pyformat first. Willing to be proven wrong :P.

I'm skeptical that this is possible without having exactly as complicated a regex. Consider that a date format could lead to the word August being rendered. In order to know that August is what you're looking for, you would need to extract their formatting code. But if you have a regex that can extract their formatting code, you may as well just use that for finding and replacing without rendering the pyformat first. Willing to be proven wrong :P.

Well you can't ask me to prove you wrong and not expect me to try 😛

def transform_paramstyle(operation: str, parameters: Dict[str, Any]) -> str:

    COMPLETE_PYFORMAT_REGEX = r'%\((\w+)\)([diouxXeEfFgGcrsa%])'
    INNER_NAME_REGEX = r'\((.*?)\)'
    NAMED_PARAMSTYLE_FMT = ":{}"

    pat = re.compile(COMPLETE_PYFORMAT_REGEX)
    inner_pat = re.compile(INNER_NAME_REGEX)

    pyformat_markers = pat.findall(operation)

    for marker in pyformat_markers:
        _name = re.findall(inner_pat, marker)[0]
        rendered_value = inject_parameters(_name, parameters.get(_name))
        operation = operation.replace(rendered_value, NAMED_PARAMSTYLE_FMT.format(_name))

    return operation

I pushed some new e2e tests of the inline ordinal parameter approach. And updated the logic that calls transform_paramstyle. Using @jadewang-db 's approach means that we now must pass a dictionary of parameters to transform_parameters. Whereas the regex approach didn't look at the parameters whatsoever.

As a consequence, transform_paramstyle won't work if cursor.execute() is passed a list of parameters. So I've revised the logic to make sure we don't do that.

One side-effect of this is that users cannot pass a List[DbsqlParameter] and also use the pyformat paramstyle. But we wouldn't expect users to do that, because DbsqlParameter itself is brand new for the forthcoming 3.0.0 release. i.e. this doesn't break backwards compatibility.