atk4 / dsql

Object-Oriented SQL Query Builder
https://agiletoolkit.org/
MIT License
58 stars 23 forks source link

[feature] introduce Expression escape constants #268

Closed georgehristov closed 4 years ago

mvorisek commented 4 years ago

As protected (ok, can be possibly used somewhere in extended classes), I think the names MUST match the constant values. Very important to prevent human conflicts.

georgehristov commented 4 years ago

Yes, the names will match the constants and now they do. If we are to change them the should be: PARAM = PARAM COMPLETE = STRING SOFT = STRING_SOFT NONE = NONE

mvorisek commented 4 years ago

Yes, the names will match the constants and now they do. If we are to change them the should be: PARAM = PARAM COMPLETE = STRING SOFT = STRING_SOFT NONE = NONE

no!

STRING (literal) === param IDENTIFIR === name

as STRING is set using bound values, yes, then PARAM is ok/good

methods should be probabaly renamed as well, as Expr::escape() is too unspecific name for identifier escaping/quoting

georgehristov commented 4 years ago

Methods is next phase - should be easy as they are protected.

Let me know your complete name mapping suggestion (CURRENT > NEW)

mvorisek commented 4 years ago

Methods is next phase - should be easy as they are protected.

Let me know your complete name mapping suggestion (CURRENT > NEW)

see updated https://github.com/atk4/dsql/pull/268#discussion_r514230598