John Russell has posted comments on this change. Change subject: Release note updates for Impala 2.8 ......................................................................
Patch Set 3: (16 comments) http://gerrit.cloudera.org:8080/#/c/5668/1//COMMIT_MSG Commit Message: Line 28: Change-Id: I7c47f422e509cec6d3eb8aaa82294b584f393aed > I won't have time to do a thorough review here but this commit message need Done http://gerrit.cloudera.org:8080/#/c/5668/1/docs/topics/impala_incompatible_changes.xml File docs/topics/impala_incompatible_changes.xml: PS1, Line 91: now > typo Done PS1, Line 111: There was a period during early experimental versions of Impala + Kudu where : non-primary-key olumns had the <codeph>NOT NULL</codeph > I don't think this is correct. AFAIK the default behavior is the same and w I clarified the wording a bit. PS1, Line 111: There was a period during early experimental versions of Impala + Kudu where : non-primary-key olumns had the <codeph>NOT NULL</codeph > The GA behavior is columns are nullable by default, unless they are part of Done http://gerrit.cloudera.org:8080/#/c/5668/3/docs/topics/impala_incompatible_changes.xml File docs/topics/impala_incompatible_changes.xml: PS3, Line 58: Impala Incompatible Changes Introduced in Impala 2.8.x > This wording is surprising to me. Why is the first word needed at all? There was something I saw relating to cross-reference links, but I think that was more applicable in Cloudera-specific docs. I removed the leading "Impala" globally. Line 65: They were output in uppercase by mistake, but only for a single Impala release (Impala 2.7). > It makes changes easier to review when the lines are 90 characters or fewer Good point. I'm going to wait and do that globally as a separate gerrit review. I suggest for entirely new text like this, go into gerrit settings and set 'left side' to 'hide'. PS3, Line 111: > space at end of line Done Line 112: non-primary-key olumns had the <codeph>NOT NULL</codeph> attribute by default. > "olumns" Done Line 1523: <concept id="incompatible_changes_07" audience="hidden"> > Why hide this? The beta releases are so intertwined with CDH and CM release numbers, I want to make a blanket rule to hide or remove such stale historical info. It is more of a Cloudera convention to let release notes accrete forever. Contrast with Apache Kudu - https://kudu.apache.org/docs/release_notes.html - where only the changes for the current release are listed in the docs for that release. http://gerrit.cloudera.org:8080/#/c/5668/1/docs/topics/impala_new_features.xml File docs/topics/impala_new_features.xml: PS1, Line 66: The <codeph>COMPUTE STATS</codeph> statement can : take advantage of multithreading. > Not sure if you want to be more specific here as the default for Parquet ta Done PS1, Line 90: > move left? You mean indentation-wise? For the XML tagging in the docs, I use Berkeley-style indenting, e.g. <p> some text </p> is the equivalent of { some C code } PS1, Line 91: > : <li> : <p rev="IMPALA-4397"> > SORTBY adds ordering for non-partition key columns to better support the ef Done PS1, Line 103: that use dynamic > move to p? Good catch. I use <p> even where optional inside list items, and prefer to apply attributes on the inner <p> tags, because there's the possibility of reusing those paragraphs verbatim in the detailed writeup of each feature. PS1, Line 238: STAMP</codeph>, <codeph>DECIMAL< > needed? Clunky wording, I'll reword just to emphasize not to worry about inserting data in small batches like we warn people about for other kinds of tables. PS1, Line 253: of the Kudu data makes it more efficient than with HDFS > dimitris should review I'll mention that Sentry can be bypassed and then have Dimitris review the subsequent patch set. PS1, Line 290: : <li> Is this purely a CM feature? If so I won't mention it in the upstream docs. -- To view, visit http://gerrit.cloudera.org:8080/5668 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I7c47f422e509cec6d3eb8aaa82294b584f393aed Gerrit-PatchSet: 3 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: John Russell <[email protected]> Gerrit-Reviewer: Alex Behm <[email protected]> Gerrit-Reviewer: Ambreen Kazi <[email protected]> Gerrit-Reviewer: Dan Hecht <[email protected]> Gerrit-Reviewer: Dimitris Tsirogiannis <[email protected]> Gerrit-Reviewer: Greg Rahn <[email protected]> Gerrit-Reviewer: Jim Apple <[email protected]> Gerrit-Reviewer: John Russell <[email protected]> Gerrit-Reviewer: Laurel Hale <[email protected]> Gerrit-Reviewer: Matthew Jacobs <[email protected]> Gerrit-HasComments: Yes
