address review comments

facebookincubator · Nov 15, 2023 · 6c348dd · 6c348dd
1 parent 35de81e
commit 6c348dd
Show file tree

Hide file tree

Showing 8 changed files with 25 additions and 10 deletions.
diff --git a/velox/type/CMakeLists.txt b/velox/type/CMakeLists.txt
@@ -15,8 +15,8 @@ if(${VELOX_BUILD_TESTING})
   add_subdirectory(tests)
 endif()
 
+add_subdirectory(parser)
 add_subdirectory(tz)
-add_subdirectory(type_parser)
 add_subdirectory(fbhive)
 
 add_library(

diff --git a/velox/type/type_parser/CMakeLists.txt → velox/type/parser/CMakeLists.txt b/velox/type/type_parser/CMakeLists.txt → velox/type/parser/CMakeLists.txt
diff --git a/velox/type/type_parser/Scanner.h → velox/type/parser/Scanner.h b/velox/type/type_parser/Scanner.h → velox/type/parser/Scanner.h
@@ -43,13 +43,13 @@ class Scanner : public yyFlexLexer {
   }
 
   // Store input to print it as part of the error message.
-  std::string input() {
+  std::string_view input() {
     return input_;
   }
 
  private:
   TypePtr& outputType_;
-  std::string input_;
+  const std::string_view input_;
 };
 
 } // namespace facebook::velox::type
diff --git a/velox/type/type_parser/TypeParser.h → velox/type/parser/TypeParser.h b/velox/type/type_parser/TypeParser.h → velox/type/parser/TypeParser.h
@@ -20,5 +20,20 @@
 #include "velox/type/Type.h"
 
 namespace facebook::velox {
+
+/// Parses a type string in Presto format to Velox type.
+/// Example type strings:
+///    row(col0 bigint, varchar)
+///    array(bigint)
+///    map(bigint, array(bigint))
+///    function(bigint,bigint,bigint)
+/// The parsing is case-insensitive. i.e. 'Row' and 'row' are equal.
+/// Field names for rows are optional.
+/// Quoted field names are supported.
+/// All types except for Presto types need to be registered. An error is thrown
+/// otherwise.
+/// Uses the Type::hasType and Type::getType APIs to convert a string to Velox
+/// type.
+
 TypePtr parseType(const std::string& typeText);
-}
+} // namespace facebook::velox
diff --git a/velox/type/type_parser/TypeParser.ll → velox/type/parser/TypeParser.ll b/velox/type/type_parser/TypeParser.ll → velox/type/parser/TypeParser.ll
@@ -2,12 +2,12 @@
 #include <vector>
 #include <memory>
 
-#include "velox/type/type_parser/TypeParser.yy.h"  // @manual
-#include "velox/type/type_parser/Scanner.h"
+#include "velox/type/parser/TypeParser.yy.h"  // @manual
+#include "velox/type/parser/Scanner.h"
 #define YY_DECL int facebook::velox::type::Scanner::lex(facebook::velox::type::Parser::semantic_type *yylval)
 %}
 
-%option c++ noyywrap noyylineno nodefault caseless
+%option c++ noyywrap noyylineno nodefault
 
 A   [A|a]
 B   [B|b]
@@ -64,7 +64,7 @@ int yyFlexLexer::yylex() {
     throw std::runtime_error("Bad call to yyFlexLexer::yylex()");
 }
 
-#include "velox/type/type_parser/TypeParser.h"
+#include "velox/type/parser/TypeParser.h"
 
 facebook::velox::TypePtr facebook::velox::parseType(const std::string& typeText)
  {

diff --git a/velox/type/type_parser/TypeParser.yy → velox/type/parser/TypeParser.yy b/velox/type/type_parser/TypeParser.yy → velox/type/parser/TypeParser.yy
@@ -32,7 +32,7 @@
 
 %code
 {
-    #include <velox/type/type_parser/Scanner.h>
+    #include <velox/type/parser/Scanner.h>
     #define yylex(x) scanner->lex(x)
     using namespace facebook::velox;
     TypePtr typeFromString(const std::string& type) {

diff --git a/velox/type/type_parser/tests/CMakeLists.txt → velox/type/parser/tests/CMakeLists.txt b/velox/type/type_parser/tests/CMakeLists.txt → velox/type/parser/tests/CMakeLists.txt
diff --git a/...type/type_parser/tests/TypeParserTest.cpp → velox/type/parser/tests/TypeParserTest.cpp b/...type/type_parser/tests/TypeParserTest.cpp → velox/type/parser/tests/TypeParserTest.cpp
@@ -20,7 +20,7 @@
 #include "velox/functions/prestosql/types/HyperLogLogType.h"
 #include "velox/functions/prestosql/types/JsonType.h"
 #include "velox/functions/prestosql/types/TimestampWithTimeZoneType.h"
-#include "velox/type/type_parser/TypeParser.h"
+#include "velox/type/parser/TypeParser.h"
 
 namespace facebook::velox {
 namespace {