{"action":"created","issue":{"url":"https://api.github.com/repos/prestodb/presto/issues/5414","repository_url":"https://api.github.com/repos/prestodb/presto","labels_url":"https://api.github.com/repos/prestodb/presto/issues/5414/labels{/name}","comments_url":"https://api.github.com/repos/prestodb/presto/issues/5414/comments","events_url":"https://api.github.com/repos/prestodb/presto/issues/5414/events","html_url":"https://github.com/prestodb/presto/pull/5414","id":159006883,"number":5414,"title":"Support for parameters in prepared statements","user":{"login":"rschlussel","id":10963789,"avatar_url":"https://avatars.githubusercontent.com/u/10963789?v=3","gravatar_id":"","url":"https://api.github.com/users/rschlussel","html_url":"https://github.com/rschlussel","followers_url":"https://api.github.com/users/rschlussel/followers","following_url":"https://api.github.com/users/rschlussel/following{/other_user}","gists_url":"https://api.github.com/users/rschlussel/gists{/gist_id}","starred_url":"https://api.github.com/users/rschlussel/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/rschlussel/subscriptions","organizations_url":"https://api.github.com/users/rschlussel/orgs","repos_url":"https://api.github.com/users/rschlussel/repos","events_url":"https://api.github.com/users/rschlussel/events{/privacy}","received_events_url":"https://api.github.com/users/rschlussel/received_events","type":"User","site_admin":false},"labels":[{"url":"https://api.github.com/repos/prestodb/presto/labels/CLA%20Signed","name":"CLA Signed","color":"ededed"}],"state":"open","locked":false,"assignee":{"login":"martint","id":14387,"avatar_url":"https://avatars.githubusercontent.com/u/14387?v=3","gravatar_id":"","url":"https://api.github.com/users/martint","html_url":"https://github.com/martint","followers_url":"https://api.github.com/users/martint/followers","following_url":"https://api.github.com/users/martint/following{/other_user}","gists_url":"https://api.github.com/users/martint/gists{/gist_id}","starred_url":"https://api.github.com/users/martint/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/martint/subscriptions","organizations_url":"https://api.github.com/users/martint/orgs","repos_url":"https://api.github.com/users/martint/repos","events_url":"https://api.github.com/users/martint/events{/privacy}","received_events_url":"https://api.github.com/users/martint/received_events","type":"User","site_admin":false},"assignees":[{"login":"martint","id":14387,"avatar_url":"https://avatars.githubusercontent.com/u/14387?v=3","gravatar_id":"","url":"https://api.github.com/users/martint","html_url":"https://github.com/martint","followers_url":"https://api.github.com/users/martint/followers","following_url":"https://api.github.com/users/martint/following{/other_user}","gists_url":"https://api.github.com/users/martint/gists{/gist_id}","starred_url":"https://api.github.com/users/martint/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/martint/subscriptions","organizations_url":"https://api.github.com/users/martint/orgs","repos_url":"https://api.github.com/users/martint/repos","events_url":"https://api.github.com/users/martint/events{/privacy}","received_events_url":"https://api.github.com/users/martint/received_events","type":"User","site_admin":false}],"milestone":null,"comments":19,"created_at":"2016-06-07T19:39:49Z","updated_at":"2016-08-10T18:48:18Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/prestodb/presto/pulls/5414","html_url":"https://github.com/prestodb/presto/pull/5414","diff_url":"https://github.com/prestodb/presto/pull/5414.diff","patch_url":"https://github.com/prestodb/presto/pull/5414.patch"},"body":"@electrum "},"comment":{"url":"https://api.github.com/repos/prestodb/presto/issues/comments/238965219","html_url":"https://github.com/prestodb/presto/pull/5414#issuecomment-238965219","issue_url":"https://api.github.com/repos/prestodb/presto/issues/5414","id":238965219,"user":{"login":"martint","id":14387,"avatar_url":"https://avatars.githubusercontent.com/u/14387?v=3","gravatar_id":"","url":"https://api.github.com/users/martint","html_url":"https://github.com/martint","followers_url":"https://api.github.com/users/martint/followers","following_url":"https://api.github.com/users/martint/following{/other_user}","gists_url":"https://api.github.com/users/martint/gists{/gist_id}","starred_url":"https://api.github.com/users/martint/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/martint/subscriptions","organizations_url":"https://api.github.com/users/martint/orgs","repos_url":"https://api.github.com/users/martint/repos","events_url":"https://api.github.com/users/martint/events{/privacy}","received_events_url":"https://api.github.com/users/martint/received_events","type":"User","site_admin":false},"created_at":"2016-08-10T18:48:17Z","updated_at":"2016-08-10T18:48:17Z","body":"> But it's much more complicated than just adding the visitParameter() method to the ExpressionAnalyzer. There are several reasons for this:\r\n> 1. The analysis is called by so many things, and the parameters need to be threaded through to all of those places. It's a pretty invasive change, though when you look at it, it's basically all the same places that you need to thread the metadata.\r\n> 2. As I had been saying earlier, the visitQualifiedNameReference method and analysis related changes take care of the analysis component, but the expressions still need to actually be rewritten for the PlanNodes. In the planner the QualifiedNameReferences get rewritten to symbols using the TranslationMap every time an expression is added to a PlanNode. For parameters I did the same, and in all places where there were calls to rewrite the expression to symbols, I added rewrites to replace Parameters as well.\r\n\r\nYes, I agree that it has a bigger surface area *today*. But, consider that as part of the new optimizer work, we're going to decouple to SQL AST from the internal representation for scalar and relational expressions. So, all uses of ExpressionAnalyzer will go away (except for the query analyzer, of course).\r\n\r\nThe flow will look like this:\r\n\r\nText --(Parser)--> SQL AST --(Analyzer)--> SQL AST + annotations --(Planner)--> IR --(Optimizer)--> Optimizer IR\r\n\r\nAnalysis of parameter types will be done during query analysis, and replacing them with the corresponding values will be done during initial planning (which will be a simple desugaring and translation from SQL AST to IR). I expect `RelationPlanner` to be vastly simplified.\r\n\r\n\r\n> 3. Parameters can appear in both regular sql statements and data definition tasks. In particular, SetSessionTask and CallTask need to replace parameters when they evaluate their arguments, and CreateViewTask needs to replace all the parameters when it calls getFormattedSql. Additionally, the explains for DataDefinitionTasks need access to parameters.\r\n\r\nDataDefinitionTasks will eventually also be represented using the same IR as everything else and go through the same optimization and execution steps. Modeling them as a entirely different concept was one of those architectural detours we had to take for expediency, but it's not an ideal state of affairs.\r\n\r\n> 4. Parameters are never equal to each other, but the values that replace them can be. This affects analysis of aggregations because you need to replace parameters there too so that you can accurately check whether a group by expression appears in the select list.\r\n\r\nThis is true, but if the parameters are semantically unrelated, it could be argued that the analysis should fail. This is similar to, say, selecting a column that's not in the group by clause but that happens to have a single constant value in the table.\r\n\r\nOne way out of this is to add support for named parameters, so users could refer to the same parameter in multiple positions in the query.\r\n\r\n"}}
{"actor":{"display_login":"martint"}}