Kevin Liew, I reviewed, and it looks great. Really good test coverage, nice clean code. Only cosmetic issues to fix:

• Can you change method javadoc comments to 3rd person declarative (e.g. prefer "Gets the label" to "Get the label");
• Remove the 3 TODOs you added. Either fix the problem, or log a jira case, or remove the comment.
• There is one checkstyle exception.

I thought that getExtendedColumns was a bit easier to understand when I added a method to transform the list of alternating names and types into a list of pairs. But it's up to you whether you take that change:

  public static List<RelDataTypeField> getExtendedColumns(
      SqlValidator validator, SqlValidatorTable table, SqlNodeList extendedColumns) {
    final ImmutableList.Builder<RelDataTypeField> extendedFields = ImmutableList.builder();
    final ExtensibleTable extTable = table.unwrap(ExtensibleTable.class);
    int extendedFieldOffset =
        extTable == null
            ? table.getRowType().getFieldCount()
            : extTable.getExtendedColumnOffset();
    for (Pair<SqlIdentifier, SqlDataTypeSpec> pair : pairs(extendedColumns)) {
      final SqlIdentifier identifier = pair.left;
      final SqlDataTypeSpec type = pair.right;
      extendedFields.add(
          new RelDataTypeFieldImpl(identifier.getSimple(),
              extendedFieldOffset++,
              type.deriveType(validator)));
    }
    return extendedFields.build();
  }

  /** Converts a list of extended columns
   * (of the form [name0, type0, name1, type1, ...])
   * into a list of (name, type) pairs. */
  private static List<Pair<SqlIdentifier, SqlDataTypeSpec>> pairs(
      SqlNodeList extendedColumns) {
    final List list = extendedColumns.getList();
    //noinspection unchecked
    return Pair.zip(Util.quotientList(list, 2, 0),
        Util.quotientList(list, 2, 1));
  }

Kevin Liew, regarding your comment

If projectMap is a SortedMap, we could use a PriorityQueue here to improve the time complexity. wdyt?

I think you can optimize

          Integer projectedOrdinal = null;
          for (Integer ordinal : modifiableViewTable.getColumnMapping()) {
            if (entry.getKey().intValue() == ordinal.intValue()) {
              projectedOrdinal = ordinal;
              break;
            }
          }
          if (projectedOrdinal == null) {
            // The constrained column was not projected in the view definition.
            continue;
          }

to something like

if (!modifiableViewTable.getColumnMapping().contains(entry.getKey())) {
  continue;
}

And if you need to intersect the sets of column ids consider creating an ImmutableBitSet for each and using ImmutableBitSet#intersects. This compares bits in order so is basically doing a "merge".

Thanks for the review Julian Hyde. I made changes based on your suggestions. I filed separate JIRA for the TODOs as they would require test cases beyond the scope of this JIRA and they aren't blocking the Phoenix-Calcite integration.

One more thing: You added quite a few imports of the rex package into SqlValidatorImpl. The validator is not supposed to depend on rex, because validation happens "before" expressions are converted to RexNode. Now, I understand that in this case the table is using RexNode to express its constraints. But still, can you think of a way to refactor this so that the RexNode dependencies don't creep into the validator? For instance, could those methods go into ModifiableViewTable? (But if so, we don't want ModifiableViewTable to depend on SqlValidator.)

I'm going to commit your change but I'd like you to refactor the code later to get the dependencies right.

I am currently making some changes based on your comments in CALCITE-1680.
I will revisit the dependency issue after giving it some thought. Thanks Julian Hyde

I ran into an issue with DDL statements (fail to validate when explicitly specifying columns of the base table in a view definition that includes extended columns).

There doesn't seem to be an easy way to add DDL tests for Calcite as I get parser errors for create view in SqlValidatorTest. Is there an existing framework for DDL tests? I can add some testing in MockCatalogReader but that doesn't seem scalable.

Nevermind about the DDL issue - I had to specify the extended column in the select list.
Though I have found another issue with columns reordered in the view. I will resolve the issue and test cases.

Here's a test for an extended parser that handles CREATE TABLE: https://github.com/apache/calcite/blob/e641211fd72a4f4a24f0f4644034c4b2f3c040ca/core/src/test/java/org/apache/calcite/sql/parser/parserextensiontesting/ExtensionSqlParserTest.java#L48

In your change "Improve time complexity for INSERT to modifiable view" the code could be made less verbose and possibly more efficient. For example indexToField.get(source.getIndex()) could be changed to indexToField.containsKey(source.getIndex()), and when you notice that you're just interested in the keys in the map, you could replace the entire method with new ImmutableBitSet(source.getKeys()). Maybe you need to intersect it with ImmutableBitSet.range(sourceRowType.getFieldCount()); I'm not sure.

Also I think getIndexToFieldMap would be more straightforward to use if it returned an immutable map, rather than populating a mutable map argument. Functional programming.

Please let me know when you've stopped editing this PR and I will review it again. Don't squash it; it's easier if I can see the incremental changes.

Thanks for the feedback Julian Hyde. The readability has improved substantially.
The PR is ready for review again.

For the future revision to insulate SqlValidatorImpl from Rex~ dependencies, I'd like to make SqlValidatorImpl.newValidationError a static method in SqlValidator so that it can be called from ModifiableViewTable. Does that sound like a good approach? Should I create a JIRA for the newValidationError changes?

I will add DDL support for CREATE VIEW to the extension parser at a later date. Is that okay?

I'd like to keep newValidationError non-static. Just in case we want to use context in the validator, such as a logger.

But I agree that it's tricky to keep the dependencies separate. How about passing in a callback (say a Function) that the ModifiableViewTable method can call on error? Or have the method populate a collection of errors?

Kevin Liew, The line Preconditions.checkArgument((table != null) == modifiable); fails in a few tests; yet the table field has the comment Not null if and only if the view is modifiable.. Is that comment no longer valid?

I made some changes to satisfy the precondition check and merged the master branch.
Working on one last change.

Please, no more changes.

Sorry Julian, I will try to focus on productivity over perfection next time. I finalized my changes.

Fixed in http://git-wip-us.apache.org/repos/asf/calcite/commit/e0a1f7d3. Thanks for the PR, Kevin Liew!

Resolved in release 1.12.0 (2017-03-24).

I refactored out some Rex~ dependencies in https://github.com/apache/calcite/pull/419

Thanks Kevin Liew, I've committed that refactoring as http://git-wip-us.apache.org/repos/asf/calcite/commit/059142fe.