Uploaded image for project: 'IMPALA'
  1. IMPALA
  2. IMPALA-5903

Inconsistent specification of result set and result set metadata

    Details

    • Type: Bug
    • Status: Open
    • Priority: Critical
    • Resolution: Unresolved
    • Affects Version/s: Impala 2.10.0
    • Fix Version/s: None
    • Component/s: Catalog, Frontend
    • Labels:
      None

      Description

      Some statements that return a result set declare it in Frontend.java, but other do not. Still the queries seem to work. We should take a closer look at how/where the result set metadata is specified and returned to the user, and ideally, make the paths today consistent (or provide a good explanation for the inconsistency).

      For example, compute stats does return a result set, but we never declare it as returning a result set.
      You can easily test the behavior of GetResultMetadata() by modifying one of our tests under tests/hs2/

      Take a look at Frontend.java and search for COMPUTE_STATS. You will see that we incorrectly declare the result set to be empty.

      From fe/src/main/java/org/apache/impala/service/Frontend.java at f8e7c31:

       403     } else if (analysis.isComputeStatsStmt()) {
       404       ddl.op_type = TCatalogOpType.DDL;
       405       TDdlExecRequest req = new TDdlExecRequest();
       406       req.setDdl_type(TDdlType.COMPUTE_STATS);
       407       req.setCompute_stats_params(analysis.getComputeStatsStmt().toThrift());
       408       ddl.setDdl_params(req);
       409       metadata.setColumns(Collections.<TColumn>emptyList());
       410     }
      

      L409 shows an empty list for the result set metadata.

      However in fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java we construct a result set:

       472         case UPDATE_STATS:
       473           Preconditions.checkState(params.isSetUpdate_stats_params());
       474           Reference<Long> numUpdatedColumns = new Reference<>(0L);
       475           alterTableUpdateStats(tbl, params.getUpdate_stats_params(), response,
       476               numUpdatedPartitions, numUpdatedColumns);
       477           reloadTableSchema = true;
       478           resultColVal.setString_val("Updated " + numUpdatedPartitions.getRef() +
       479               " partition(s) and " + numUpdatedColumns.getRef() + " column(s).");
       480           setResultSet = true;
       481           break;
      

      This has the potential to break HS2 clients that expect a result set when computing stats. But in practice, compute stats works and returns a result set, so the current code/flow is confusing.

        Issue Links

          Activity

          Hide
          dhecht Dan Hecht added a comment -

          Alexander Behm What could be the fallout from fixing this? Or is it pretty risk free?

          Show
          dhecht Dan Hecht added a comment - Alexander Behm What could be the fallout from fixing this? Or is it pretty risk free?
          Hide
          alex.behm Alexander Behm added a comment -

          Dan Hecht I more concretely nailed down the original issue that prompted this JIRA, but the weirdness in this JIRA is still valid. This JIRA is more exploratory and somebody should investigate and hopefully fix the inconsistencies in result set declaration. I am not sure what the fallout is without doing that investigation.

          Show
          alex.behm Alexander Behm added a comment - Dan Hecht I more concretely nailed down the original issue that prompted this JIRA, but the weirdness in this JIRA is still valid. This JIRA is more exploratory and somebody should investigate and hopefully fix the inconsistencies in result set declaration. I am not sure what the fallout is without doing that investigation.

            People

            • Assignee:
              Unassigned
              Reporter:
              lv Lars Volker
            • Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

              • Created:
                Updated:

                Development