impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Matthew Jacobs (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-5784 : Separate planner and user set query options in profile
Date Tue, 22 Aug 2017 21:58:51 GMT
Matthew Jacobs has posted comments on this change.

Change subject: IMPALA-5784 : Separate planner and user set query options in profile
......................................................................


Patch Set 1:

> > > (1 comment)
 > > Relying to Dan and Balasz:
 > >
 > > It's actually hard to determine what gets set where using the
 > > QueryOptions map thing we have right now, mostly because of how
 > > inconsistent we are with default values. E.g. what if a property
 > is
 > > set in the session, then the planner sets it as well, but happens
 > > to set it to the default value. Right now we can't even identify
 > > that as a non-default query option.
 > >
 > 
 > I don't follow this. We're defining "non-default" query option to
 > mean everything that doesn't match the static query option value,
 > right?
 > 
 > If so, why can't we just compare the query option values after
 > planning and to the values before planning, and print any
 > differences as "planner set" (or whatever name).  And then compare
 > the query options before planning with the static defaults, remove
 > anything we've already printed as "planner set" and print those
 > remaining differences as "manually set".
 > 
 > or are you saying that doing that is tricky due to the
 > datastructures we currently have?

Yes, I'm saying the data structures we have right now don't lend themselves to what you're
describing. Its certainly possible, but it'd be awkward given how it currently works and I
think it'd be kind of a pain to test and make sure we don't break different cases (e.g. values
set in the session/query x modified or not by the planner x with/without a static default).
If you think it's worth it in terms of usability, then Bikram can keep exploring that path.

-- 
To view, visit http://gerrit.cloudera.org:8080/7721
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ibfc7832963fa0bd278a45c06a5a54e1bf40d8876
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Bikramjeet Vig <bikramjeet.vig@cloudera.com>
Gerrit-Reviewer: Balazs Jeszenszky <jeszyb@gmail.com>
Gerrit-Reviewer: Bikramjeet Vig <bikramjeet.vig@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dhecht@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj@cloudera.com>
Gerrit-HasComments: No

Mime
View raw message