ocket8888 commented on code in PR #7099: URL: https://github.com/apache/trafficcontrol/pull/7099#discussion_r1025603512
########## 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: > Changing all of the Go occurrences in one place would still be possible Sure, but doing that yields a fundamentally broken piece of software, so idk how useful that is. > 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. Without looking at those concatenations, I would say I probably disagree with their use as well, since I know such instances exist. > 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. I think that's actually probably the best course, is to pass it in for statement preparation. That seems more proper than string concatenation and makes the connection between the Go constant and the database enum even more clear. So that's what I'll do. -- 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]
