[impala] 02/02: IMPALA-8557: Add '.txt' to text files, remove '.' at end of filenames

[st...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

stakiar pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git

commit 8b8a49e617818e9bcf99b784b63587c95cebd622
Author: Sahil Takiar <ta...@gmail.com>
AuthorDate: Fri Nov 1 20:30:34 2019 +0000

    IMPALA-8557: Add '.txt' to text files, remove '.' at end of filenames
    
    Writes to text tables on ABFS are failing because HADOOP-15860 recently
    changed the ABFS behavior when writing files / folders that end with a
    '.'. ABFS explicitly does not allow files / folders that end with a dot.
    From the ABFS docs: "Avoid blob names that end with a dot (.), a forward
    slash (/), or a sequence or combination of the two."
    
    The behavior prior to HADOOP-15860 was to simply drop any trailing dots
    when writing files or folders, but that can lead to various issues
    because clients may try to read back a file that should exist on ABFS,
    but doesn't. HADOOP-15860 changed the behavior so that any attempt to
    write a file or folder with a trailing dot fails on ABFS.
    
    Impala writes all text files with a trailing dot due to some odd
    behavior in hdfs-table-sink.cc. The table sink writes files with
    a "file extension" which is dependent on the file type. For example,
    Parquet files have a file extension of ".parq". For some reason, text
    files had no file extension, so Impala would try to write text files of
    the following form:
    "244c5ee8ece6f759-8b1a1e3b00000000_45513034_data.0.".
    
    Several tables created during dataload, such as alltypes, already use
    the '.txt' extension for their files. These tables are not created via
    Impala's INSERT code path, they are copied into the table. However,
    there are several tables created during dataload, such as
    alltypesinsert, that are created via Impala. This patch will change
    the files in these tables so that they end in '.txt'.
    
    This patch adds the ".txt" extension to all written text files and
    modifies the hdfs-table-sink.cc so that it doesn't add a trailing dot to
    a filename if there is no file extension.
    
    Testing:
    * Ran core tests
    * Re-ran affected ABFS tests
    * Added test to validate that the correct file extension is used for
    Parquet and text tables
    * Manually validated that without the addition of the '.txt' file
    extension, files are not written with a trailing dot
    
    Change-Id: I2a9adacd45855cde86724e10f8a131e17ebf46f8
    Reviewed-on: http://gerrit.cloudera.org:8080/14621
    Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
    Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
 be/src/exec/hdfs-table-sink.cc        |  6 ++++--
 be/src/exec/hdfs-text-table-writer.cc |  2 +-
 tests/query_test/test_insert.py       | 30 ++++++++++++++++++++++++++++++
 3 files changed, 35 insertions(+), 3 deletions(-)

diff --git a/be/src/exec/hdfs-table-sink.cc b/be/src/exec/hdfs-table-sink.cc
index ea53d1a..3e695b4 100644
--- a/be/src/exec/hdfs-table-sink.cc
+++ b/be/src/exec/hdfs-table-sink.cc
@@ -340,7 +340,9 @@ Status HdfsTableSink::WriteClusteredRowBatch(RuntimeState* state, RowBatch* batc
 Status HdfsTableSink::CreateNewTmpFile(RuntimeState* state,
     OutputPartition* output_partition) {
   SCOPED_TIMER(ADD_TIMER(profile(), "TmpFileCreateTimer"));
-  string final_location = Substitute("$0.$1.$2",
+  string file_name_pattern =
+      output_partition->writer->file_extension().empty() ? "$0.$1" : "$0.$1.$2";
+  string final_location = Substitute(file_name_pattern,
       output_partition->final_hdfs_file_name_prefix, output_partition->num_files,
       output_partition->writer->file_extension());
 
@@ -351,7 +353,7 @@ Status HdfsTableSink::CreateNewTmpFile(RuntimeState* state,
   if (ShouldSkipStaging(state, output_partition)) {
     output_partition->current_file_name = final_location;
   } else {
-    output_partition->current_file_name = Substitute("$0.$1.$2",
+    output_partition->current_file_name = Substitute(file_name_pattern,
       output_partition->tmp_hdfs_file_name_prefix, output_partition->num_files,
       output_partition->writer->file_extension());
   }
diff --git a/be/src/exec/hdfs-text-table-writer.cc b/be/src/exec/hdfs-text-table-writer.cc
index f09b161..6b77852 100644
--- a/be/src/exec/hdfs-text-table-writer.cc
+++ b/be/src/exec/hdfs-text-table-writer.cc
@@ -62,7 +62,7 @@ void HdfsTextTableWriter::Close() {
 
 uint64_t HdfsTextTableWriter::default_block_size() const { return 0; }
 
-string HdfsTextTableWriter::file_extension() const { return ""; }
+string HdfsTextTableWriter::file_extension() const { return "txt"; }
 
 Status HdfsTextTableWriter::AppendRows(
     RowBatch* batch, const vector<int32_t>& row_group_indices, bool* new_file) {
diff --git a/tests/query_test/test_insert.py b/tests/query_test/test_insert.py
index 7807030..edbf853 100644
--- a/tests/query_test/test_insert.py
+++ b/tests/query_test/test_insert.py
@@ -309,3 +309,33 @@ class TestInsertNullQueries(ImpalaTestSuite):
   @pytest.mark.execute_serially
   def test_insert_null(self, vector):
     self.run_test_case('QueryTest/insert_null', vector)
+
+
+class TestInsertFileExtension(ImpalaTestSuite):
+  """Tests that files written to a table have the correct file extension. Asserts that
+  Parquet files end with .parq and text files end with .txt."""
+
+  @classmethod
+  def get_workload(self):
+    return 'functional-query'
+
+  @classmethod
+  def add_test_dimensions(cls):
+    cls.ImpalaTestMatrix.add_dimension(ImpalaTestDimension(
+        'table_format_and_file_extension',
+        *[('parquet', '.parq'), ('textfile', '.txt')]))
+
+  @classmethod
+  def setup_class(cls):
+    super(TestInsertFileExtension, cls).setup_class()
+
+  def test_file_extension(self, vector, unique_database):
+    table_format = vector.get_value('table_format_and_file_extension')[0]
+    file_extension = vector.get_value('table_format_and_file_extension')[1]
+    table_name = "{0}_table".format(table_format)
+    ctas_query = "create table {0}.{1} stored as {2} as select 1".format(
+        unique_database, table_name, table_format)
+    self.execute_query_expect_success(self.client, ctas_query)
+    for path in self.filesystem_client.ls("test-warehouse/{0}.db/{1}".format(
+        unique_database, table_name)):
+      if not path.startswith('_'): assert path.endswith(file_extension)