Add support OrcReader based on DwrfReader

[wypb]

The ORC file format is used by many companies. Currently, the DWRF Reader in Velox can be used to read ORC files. This PR implements OrcReaderFactory based on DwrfReader and registers it in HiveConnectorFactory#initialize(). In this way, we can get a Reader that can read the ORC file format through dwio::common::getReaderFactory(FileFormat::ORC)->createReader(..).

CC: @Yuhta

[netlify]

✅ Deploy Preview for meta-velox canceled.

Name	Link
🔨 Latest commit	05f6027
🔍 Latest deploy log	https://app.netlify.com/sites/meta-velox/deploys/66723655850bb00008243a56

[facebook-github-bot]

@kagamiori has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

[kagamiori]

Hi @wypb, there are a few unit test failing internally. An example failure is

[ RUN      ] OrcReaderTest.testOrcReadAllType

E0618 14:16:53.845489 315752 Exceptions.h:67] Line: fbcode/velox/common/file/File.cpp:108, Function:LocalReadFile, Expression:  No such file or directory: /data/sandcastle/boxes/eden-trunk-hg-full-fbsource/fbcode/velox/dwio/dwrf/test/examples/orc_all_type.orc, Source: RUNTIME, ErrorCode: FILE_NOT_FOUND
terminate called after throwing an instance of 'facebook::velox::VeloxRuntimeError'
  what():  Exception: VeloxRuntimeError
Error Source: RUNTIME
Error Code: FILE_NOT_FOUND
Reason: No such file or directory: /data/sandcastle/boxes/eden-trunk-hg-full-fbsource/fbcode/velox/dwio/dwrf/test/examples/orc_all_type.orc
Retriable: False
Function: LocalReadFile
File: fbcode/velox/common/file/File.cpp
Line: 108

I wonder whether we need to update the getExampleFilePath() method used in the unit tests. Also wondering why these unit tests do not fail in CI here. Could you take a look?

[wypb]

Hi @kagamiori Thank you for you review.
I checked the reason. CI here will not fail because current_path does not end with fbcode. Meta internally fails because current_path ends with fbcode, so velox/dwio/dwrf/test is added to the file path.

std::string getDataFilePath(
    const std::string& baseDir,
    const std::string& filePath) {
  std::string current_path = fs::current_path().c_str();
  if (boost::algorithm::ends_with(current_path, "fbcode")) {
    return current_path + "/" + baseDir + "/" + filePath;
  }
  return current_path + "/" + filePath;
}

The getExampleFilePath method should be rewritten in orc test. The baseDir parameter of the getDataFilePath method needs to be set to velox/dwio/orc/test.

[facebook-github-bot]

@kagamiori has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

[facebook-github-bot]

@kagamiori merged this pull request in 1a50a8a.

[kagamiori]

Hi @kagamiori Thank you for you review. I checked the reason. CI here will not fail because current_path does not end with fbcode. Meta internally fails because current_path ends with fbcode, so velox/dwio/dwrf/test is added to the file path.

std::string getDataFilePath(
    const std::string& baseDir,
    const std::string& filePath) {
  std::string current_path = fs::current_path().c_str();
  if (boost::algorithm::ends_with(current_path, "fbcode")) {
    return current_path + "/" + baseDir + "/" + filePath;
  }
  return current_path + "/" + filePath;
}

The getExampleFilePath method should be rewritten in orc test. The baseDir parameter of the getDataFilePath method needs to be set to velox/dwio/orc/test.

Wow, I didn't even know we hard-code "fbcode" in our code...Thank you for looking into it and fix the unit test!

[conbench-facebook]

Conbench analyzed the 1 benchmark run on commit 1a50a8af.

There were no benchmark performance regressions. 🎉

The full Conbench report has more details.