zrhoffman commented on code in PR #7099: URL: https://github.com/apache/trafficcontrol/pull/7099#discussion_r1025592447
########## traffic_ops/traffic_ops_golang/crconfig/deliveryservice.go: ########## @@ -145,7 +145,7 @@ FROM deliveryservice AS d INNER JOIN type AS t ON t.id = d.type LEFT OUTER JOIN profile AS p ON p.id = d.profile WHERE d.cdn_id = (select id FROM cdn WHERE name = $1) -AND d.active = true +AND d.active = 'ACTIVE' Review Comment: > And also if the string used to represent "Active" ever changes, it's not possible to change it in only one place, since it would require a database migration Changing all of the Go occurrences in one place would still be possible > Plus, using string concatenation for building SQL queries - besides always making me a little nervous inherently - means that knowing what you're doing is safe depends on knowing the exact string value of the constant, so an abstraction could be detrimental to that. 3 of the 6 queries I commented on are queries that already use concatenation, so concatenation should work there, as well as the 2 query fragments. For the 3 queries that don't use concatenation, passing `tc.DSActiveStateActive` as a parameter to the prepared statement would work (and, unlike the other cases, would not require a typecast). That said, if you feel that it's more maintainable as-is, I'm fine with you keeping it like that. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
