----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57876/#review170141 -----------------------------------------------------------
Ship it! Hey Szabi, Although your solution for the problem seems to be right and the testing is also quite exhaustive (nice job!), I would consider opening a new FR JIRA for eliminating the problems root cause by forbidding $ signs in the type names (originated from table names) with the same conversion Sqoop already does in case of several other special characters. My 2cents, Attila - Attila Szabo On March 24, 2017, 1:20 p.m., Szabolcs Vasas wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/57876/ > ----------------------------------------------------------- > > (Updated March 24, 2017, 1:20 p.m.) > > > Review request for Sqoop. > > > Bugs: SQOOP-3159 > https://issues.apache.org/jira/browse/SQOOP-3159 > > > Repository: sqoop-trunk > > > Description > ------- > > The fix itself could be done by rewriting line > org/apache/sqoop/orm/CompilationManager.java:303 from > > int indexOfDollarSign = chompedPath.indexOf("$"); > > to > > int indexOfDollarSign = chompedPath.lastIndexOf("$"); > > because that block is supposed to get the outer class name from the class > name but a class name could contain a $ sign as well. > So for example the outer class name of class Foo$Bar$1 is be Foo$Bar. > > However to make the unit testing easier and make the purpose of that method > clearer I did some refactoring in > org.apache.sqoop.orm.CompilationManager#addClassFilesFromDir method. > > > Diffs > ----- > > src/java/org/apache/sqoop/orm/CompilationManager.java c1a656b > src/test/com/cloudera/sqoop/TestExport.java df5a663 > src/test/com/cloudera/sqoop/manager/OracleExportTest.java ec56cbe > > src/test/org/apache/sqoop/manager/oracle/OracleSpecialCharacterTableImportTest.java > PRE-CREATION > src/test/org/apache/sqoop/orm/TestCompilationManager.java PRE-CREATION > > > Diff: https://reviews.apache.org/r/57876/diff/3/ > > > Testing > ------- > > ant clean test > > New unit tests and third party tests are added and ran successfully. > > > Thanks, > > Szabolcs Vasas > >
