You are viewing a plain text version of this content. The canonical link for it is here.
Posted to mapreduce-commits@hadoop.apache.org by to...@apache.org on 2009/12/17 22:31:09 UTC
svn commit: r891920 - in /hadoop/mapreduce/trunk: ./
src/contrib/sqoop/src/java/org/apache/hadoop/sqoop/manager/
src/contrib/sqoop/src/java/org/apache/hadoop/sqoop/mapreduce/
src/contrib/sqoop/src/test/org/apache/hadoop/sqoop/manager/
Author: tomwhite
Date: Thu Dec 17 21:31:08 2009
New Revision: 891920
URL: http://svn.apache.org/viewvc?rev=891920&view=rev
Log:
MAPREDUCE-1174. Sqoop improperly handles table/column names which are reserved sql words. Contributed by Aaron Kimball.
Modified:
hadoop/mapreduce/trunk/CHANGES.txt
hadoop/mapreduce/trunk/src/contrib/sqoop/src/java/org/apache/hadoop/sqoop/manager/ConnManager.java
hadoop/mapreduce/trunk/src/contrib/sqoop/src/java/org/apache/hadoop/sqoop/manager/MySQLManager.java
hadoop/mapreduce/trunk/src/contrib/sqoop/src/java/org/apache/hadoop/sqoop/manager/OracleManager.java
hadoop/mapreduce/trunk/src/contrib/sqoop/src/java/org/apache/hadoop/sqoop/manager/PostgresqlManager.java
hadoop/mapreduce/trunk/src/contrib/sqoop/src/java/org/apache/hadoop/sqoop/manager/SqlManager.java
hadoop/mapreduce/trunk/src/contrib/sqoop/src/java/org/apache/hadoop/sqoop/mapreduce/DataDrivenImportJob.java
hadoop/mapreduce/trunk/src/contrib/sqoop/src/test/org/apache/hadoop/sqoop/manager/LocalMySQLTest.java
hadoop/mapreduce/trunk/src/contrib/sqoop/src/test/org/apache/hadoop/sqoop/manager/PostgresqlTest.java
Modified: hadoop/mapreduce/trunk/CHANGES.txt
URL: http://svn.apache.org/viewvc/hadoop/mapreduce/trunk/CHANGES.txt?rev=891920&r1=891919&r2=891920&view=diff
==============================================================================
--- hadoop/mapreduce/trunk/CHANGES.txt (original)
+++ hadoop/mapreduce/trunk/CHANGES.txt Thu Dec 17 21:31:08 2009
@@ -153,6 +153,9 @@
MAPREDUCE-1146. Sqoop dependencies break Eclipse build on Linux.
(Aaron Kimball via tomwhite)
+ MAPREDUCE-1174. Sqoop improperly handles table/column names which are
+ reserved sql words. (Aaron Kimball via tomwhite)
+
Release 0.21.0 - Unreleased
INCOMPATIBLE CHANGES
Modified: hadoop/mapreduce/trunk/src/contrib/sqoop/src/java/org/apache/hadoop/sqoop/manager/ConnManager.java
URL: http://svn.apache.org/viewvc/hadoop/mapreduce/trunk/src/contrib/sqoop/src/java/org/apache/hadoop/sqoop/manager/ConnManager.java?rev=891920&r1=891919&r2=891920&view=diff
==============================================================================
--- hadoop/mapreduce/trunk/src/contrib/sqoop/src/java/org/apache/hadoop/sqoop/manager/ConnManager.java (original)
+++ hadoop/mapreduce/trunk/src/contrib/sqoop/src/java/org/apache/hadoop/sqoop/manager/ConnManager.java Thu Dec 17 21:31:08 2009
@@ -95,6 +95,30 @@
throws IOException, ImportException;
/**
+ * When using a column name in a generated SQL query, how (if at all)
+ * should we escape that column name? e.g., a column named "table"
+ * may need to be quoted with backtiks: "`table`".
+ *
+ * @param colName the column name as provided by the user, etc.
+ * @return how the column name should be rendered in the sql text.
+ */
+ public String escapeColName(String colName) {
+ return colName;
+ }
+
+ /**
+ * When using a table name in a generated SQL query, how (if at all)
+ * should we escape that column name? e.g., a table named "table"
+ * may need to be quoted with backtiks: "`table`".
+ *
+ * @param tableName the table name as provided by the user, etc.
+ * @return how the table name should be rendered in the sql text.
+ */
+ public String escapeTableName(String tableName) {
+ return tableName;
+ }
+
+ /**
* Perform any shutdown operations on the connection.
*/
public abstract void close() throws SQLException;
Modified: hadoop/mapreduce/trunk/src/contrib/sqoop/src/java/org/apache/hadoop/sqoop/manager/MySQLManager.java
URL: http://svn.apache.org/viewvc/hadoop/mapreduce/trunk/src/contrib/sqoop/src/java/org/apache/hadoop/sqoop/manager/MySQLManager.java?rev=891920&r1=891919&r2=891920&view=diff
==============================================================================
--- hadoop/mapreduce/trunk/src/contrib/sqoop/src/java/org/apache/hadoop/sqoop/manager/MySQLManager.java (original)
+++ hadoop/mapreduce/trunk/src/contrib/sqoop/src/java/org/apache/hadoop/sqoop/manager/MySQLManager.java Thu Dec 17 21:31:08 2009
@@ -56,7 +56,7 @@
@Override
protected String getColNamesQuery(String tableName) {
// Use mysql-specific hints and LIMIT to return fast
- return "SELECT t.* FROM " + tableName + " AS t LIMIT 1";
+ return "SELECT t.* FROM " + escapeTableName(tableName) + " AS t LIMIT 1";
}
@Override
@@ -141,5 +141,35 @@
LOG.info("Executing SQL statement: " + stmt);
return statement.executeQuery();
}
+
+ /**
+ * When using a column name in a generated SQL query, how (if at all)
+ * should we escape that column name? e.g., a column named "table"
+ * may need to be quoted with backtiks: "`table`".
+ *
+ * @param colName the column name as provided by the user, etc.
+ * @return how the column name should be rendered in the sql text.
+ */
+ public String escapeColName(String colName) {
+ if (null == colName) {
+ return null;
+ }
+ return "`" + colName + "`";
+ }
+
+ /**
+ * When using a table name in a generated SQL query, how (if at all)
+ * should we escape that column name? e.g., a table named "table"
+ * may need to be quoted with backtiks: "`table`".
+ *
+ * @param tableName the table name as provided by the user, etc.
+ * @return how the table name should be rendered in the sql text.
+ */
+ public String escapeTableName(String tableName) {
+ if (null == tableName) {
+ return null;
+ }
+ return "`" + tableName + "`";
+ }
}
Modified: hadoop/mapreduce/trunk/src/contrib/sqoop/src/java/org/apache/hadoop/sqoop/manager/OracleManager.java
URL: http://svn.apache.org/viewvc/hadoop/mapreduce/trunk/src/contrib/sqoop/src/java/org/apache/hadoop/sqoop/manager/OracleManager.java?rev=891920&r1=891919&r2=891920&view=diff
==============================================================================
--- hadoop/mapreduce/trunk/src/contrib/sqoop/src/java/org/apache/hadoop/sqoop/manager/OracleManager.java (original)
+++ hadoop/mapreduce/trunk/src/contrib/sqoop/src/java/org/apache/hadoop/sqoop/manager/OracleManager.java Thu Dec 17 21:31:08 2009
@@ -50,7 +50,7 @@
protected String getColNamesQuery(String tableName) {
// SqlManager uses "tableName AS t" which doesn't work in Oracle.
- return "SELECT t.* FROM " + tableName + " t";
+ return "SELECT t.* FROM " + escapeTableName(tableName) + " t";
}
/**
Modified: hadoop/mapreduce/trunk/src/contrib/sqoop/src/java/org/apache/hadoop/sqoop/manager/PostgresqlManager.java
URL: http://svn.apache.org/viewvc/hadoop/mapreduce/trunk/src/contrib/sqoop/src/java/org/apache/hadoop/sqoop/manager/PostgresqlManager.java?rev=891920&r1=891919&r2=891920&view=diff
==============================================================================
--- hadoop/mapreduce/trunk/src/contrib/sqoop/src/java/org/apache/hadoop/sqoop/manager/PostgresqlManager.java (original)
+++ hadoop/mapreduce/trunk/src/contrib/sqoop/src/java/org/apache/hadoop/sqoop/manager/PostgresqlManager.java Thu Dec 17 21:31:08 2009
@@ -67,7 +67,7 @@
@Override
protected String getColNamesQuery(String tableName) {
// Use LIMIT to return fast
- return "SELECT t.* FROM " + tableName + " AS t LIMIT 1";
+ return "SELECT t.* FROM " + escapeTableName(tableName) + " AS t LIMIT 1";
}
@Override
Modified: hadoop/mapreduce/trunk/src/contrib/sqoop/src/java/org/apache/hadoop/sqoop/manager/SqlManager.java
URL: http://svn.apache.org/viewvc/hadoop/mapreduce/trunk/src/contrib/sqoop/src/java/org/apache/hadoop/sqoop/manager/SqlManager.java?rev=891920&r1=891919&r2=891920&view=diff
==============================================================================
--- hadoop/mapreduce/trunk/src/contrib/sqoop/src/java/org/apache/hadoop/sqoop/manager/SqlManager.java (original)
+++ hadoop/mapreduce/trunk/src/contrib/sqoop/src/java/org/apache/hadoop/sqoop/manager/SqlManager.java Thu Dec 17 21:31:08 2009
@@ -68,7 +68,7 @@
*/
protected String getColNamesQuery(String tableName) {
// adding where clause to prevent loading a big table
- return "SELECT t.* FROM " + tableName + " AS t WHERE 1=0";
+ return "SELECT t.* FROM " + escapeTableName(tableName) + " AS t WHERE 1=0";
}
@Override
@@ -170,13 +170,13 @@
if (!first) {
sb.append(", ");
}
- sb.append(col);
+ sb.append(escapeColName(col));
first = false;
}
sb.append(" FROM ");
- sb.append(tableName);
+ sb.append(escapeTableName(tableName));
sb.append(" AS "); // needed for hsqldb; doesn't hurt anyone else.
- sb.append(tableName);
+ sb.append(escapeTableName(tableName));
return execute(sb.toString());
}
Modified: hadoop/mapreduce/trunk/src/contrib/sqoop/src/java/org/apache/hadoop/sqoop/mapreduce/DataDrivenImportJob.java
URL: http://svn.apache.org/viewvc/hadoop/mapreduce/trunk/src/contrib/sqoop/src/java/org/apache/hadoop/sqoop/mapreduce/DataDrivenImportJob.java?rev=891920&r1=891919&r2=891920&view=diff
==============================================================================
--- hadoop/mapreduce/trunk/src/contrib/sqoop/src/java/org/apache/hadoop/sqoop/mapreduce/DataDrivenImportJob.java (original)
+++ hadoop/mapreduce/trunk/src/contrib/sqoop/src/java/org/apache/hadoop/sqoop/mapreduce/DataDrivenImportJob.java Thu Dec 17 21:31:08 2009
@@ -149,14 +149,23 @@
colNames = mgr.getColumnNames(tableName);
}
+ String [] sqlColNames = null;
+ if (null != colNames) {
+ sqlColNames = new String[colNames.length];
+ for (int i = 0; i < colNames.length; i++) {
+ sqlColNames[i] = mgr.escapeColName(colNames[i]);
+ }
+ }
+
// It's ok if the where clause is null in DBInputFormat.setInput.
String whereClause = options.getWhereClause();
// We can't set the class properly in here, because we may not have the
// jar loaded in this JVM. So we start by calling setInput() with DBWritable,
// and then overriding the string manually.
- DataDrivenDBInputFormat.setInput(job, DBWritable.class, tableName, whereClause,
- splitByCol, colNames);
+ DataDrivenDBInputFormat.setInput(job, DBWritable.class,
+ mgr.escapeTableName(tableName), whereClause,
+ mgr.escapeColName(splitByCol), sqlColNames);
job.getConfiguration().set(DBConfiguration.INPUT_CLASS_PROPERTY, tableClassName);
PerfCounters counters = new PerfCounters();
Modified: hadoop/mapreduce/trunk/src/contrib/sqoop/src/test/org/apache/hadoop/sqoop/manager/LocalMySQLTest.java
URL: http://svn.apache.org/viewvc/hadoop/mapreduce/trunk/src/contrib/sqoop/src/test/org/apache/hadoop/sqoop/manager/LocalMySQLTest.java?rev=891920&r1=891919&r2=891920&view=diff
==============================================================================
--- hadoop/mapreduce/trunk/src/contrib/sqoop/src/test/org/apache/hadoop/sqoop/manager/LocalMySQLTest.java (original)
+++ hadoop/mapreduce/trunk/src/contrib/sqoop/src/test/org/apache/hadoop/sqoop/manager/LocalMySQLTest.java Thu Dec 17 21:31:08 2009
@@ -38,6 +38,7 @@
import org.apache.hadoop.fs.Path;
import org.apache.hadoop.io.IOUtils;
import org.apache.hadoop.sqoop.SqoopOptions;
+import org.apache.hadoop.sqoop.testutil.CommonArgs;
import org.apache.hadoop.sqoop.testutil.ImportJobTestCase;
import org.apache.hadoop.sqoop.util.FileListing;
@@ -181,19 +182,21 @@
}
}
- private String [] getArgv(boolean mysqlOutputDelims, String... extraArgs) {
+ private String [] getArgv(boolean mysqlOutputDelims, boolean isDirect,
+ String tableName, String... extraArgs) {
ArrayList<String> args = new ArrayList<String>();
- args.add("-D");
- args.add("fs.default.name=file:///");
+ CommonArgs.addHadoopFlags(args);
args.add("--table");
- args.add(TABLE_NAME);
+ args.add(tableName);
args.add("--warehouse-dir");
args.add(getWarehouseDir());
args.add("--connect");
args.add(CONNECT_STRING);
- args.add("--direct");
+ if (isDirect) {
+ args.add("--direct");
+ }
args.add("--username");
args.add(getCurrentUser());
args.add("--where");
@@ -214,12 +217,19 @@
return args.toArray(new String[0]);
}
- private void doLocalBulkImport(boolean mysqlOutputDelims,
- String [] expectedResults, String [] extraArgs) throws IOException {
+ private void doImport(boolean mysqlOutputDelims, boolean isDirect,
+ String tableName, String [] expectedResults, String [] extraArgs)
+ throws IOException {
Path warehousePath = new Path(this.getWarehouseDir());
- Path tablePath = new Path(warehousePath, TABLE_NAME);
- Path filePath = new Path(tablePath, "data-00000");
+ Path tablePath = new Path(warehousePath, tableName);
+
+ Path filePath;
+ if (isDirect) {
+ filePath = new Path(tablePath, "data-00000");
+ } else {
+ filePath = new Path(tablePath, "part-m-00000");
+ }
File tableFile = new File(tablePath.toString());
if (tableFile.exists() && tableFile.isDirectory()) {
@@ -227,7 +237,7 @@
FileListing.recursiveDeleteDir(tableFile);
}
- String [] argv = getArgv(mysqlOutputDelims, extraArgs);
+ String [] argv = getArgv(mysqlOutputDelims, isDirect, tableName, extraArgs);
try {
runImport(argv);
} catch (IOException ioe) {
@@ -237,7 +247,7 @@
}
File f = new File(filePath.toString());
- assertTrue("Could not find imported data file", f.exists());
+ assertTrue("Could not find imported data file: " + f, f.exists());
BufferedReader r = null;
try {
// Read through the file and make sure it's all there.
@@ -262,7 +272,7 @@
"3,Fred,2009-01-23,15,marketing"
};
- doLocalBulkImport(false, expectedResults, null);
+ doImport(false, true, TABLE_NAME, expectedResults, null);
}
@Test
@@ -275,7 +285,7 @@
String [] extraArgs = { "-", "--lock-tables" };
- doLocalBulkImport(false, expectedResults, extraArgs);
+ doImport(false, true, TABLE_NAME, expectedResults, extraArgs);
}
@Test
@@ -286,6 +296,110 @@
"3,'Fred','2009-01-23',15,'marketing'"
};
- doLocalBulkImport(true, expectedResults, null);
+ doImport(true, true, TABLE_NAME, expectedResults, null);
+ }
+
+ @Test
+ public void testMysqlJdbcImport() throws IOException {
+ String [] expectedResults = {
+ "2,Bob,2009-04-20,400.0,sales",
+ "3,Fred,2009-01-23,15.0,marketing"
+ };
+
+ doImport(false, false, TABLE_NAME, expectedResults, null);
+ }
+
+ @Test
+ public void testJdbcEscapedTableName() throws Exception {
+ // Test a JDBC-based import of a table whose name is
+ // a reserved sql keyword (and is thus `quoted`)
+ final String reservedTableName = "TABLE";
+ SqoopOptions options = new SqoopOptions(CONNECT_STRING,
+ reservedTableName);
+ options.setUsername(getCurrentUser());
+ ConnManager mgr = new MySQLManager(options);
+
+ Connection connection = null;
+ Statement st = null;
+
+ try {
+ connection = mgr.getConnection();
+ connection.setAutoCommit(false);
+ st = connection.createStatement();
+
+ // create the database table and populate it with data.
+ st.executeUpdate("DROP TABLE IF EXISTS `" + reservedTableName + "`");
+ st.executeUpdate("CREATE TABLE `" + reservedTableName + "` ("
+ + "id INT NOT NULL PRIMARY KEY AUTO_INCREMENT, "
+ + "name VARCHAR(24) NOT NULL, "
+ + "start_date DATE, "
+ + "salary FLOAT, "
+ + "dept VARCHAR(32))");
+
+ st.executeUpdate("INSERT INTO `" + reservedTableName + "` VALUES("
+ + "2,'Aaron','2009-05-14',1000000.00,'engineering')");
+ connection.commit();
+ } finally {
+ if (null != st) {
+ st.close();
+ }
+
+ if (null != connection) {
+ connection.close();
+ }
+ }
+
+ String [] expectedResults = {
+ "2,Aaron,2009-05-14,1000000.0,engineering"
+ };
+
+ doImport(false, false, reservedTableName, expectedResults, null);
+ }
+
+ @Test
+ public void testJdbcEscapedColumnName() throws Exception {
+ // Test a JDBC-based import of a table with a column whose name is
+ // a reserved sql keyword (and is thus `quoted`)
+ final String tableName = "mysql_escaped_col_table";
+ SqoopOptions options = new SqoopOptions(CONNECT_STRING,
+ tableName);
+ options.setUsername(getCurrentUser());
+ ConnManager mgr = new MySQLManager(options);
+
+ Connection connection = null;
+ Statement st = null;
+
+ try {
+ connection = mgr.getConnection();
+ connection.setAutoCommit(false);
+ st = connection.createStatement();
+
+ // create the database table and populate it with data.
+ st.executeUpdate("DROP TABLE IF EXISTS " + tableName);
+ st.executeUpdate("CREATE TABLE " + tableName + " ("
+ + "id INT NOT NULL PRIMARY KEY AUTO_INCREMENT, "
+ + "`table` VARCHAR(24) NOT NULL, "
+ + "`CREATE` DATE, "
+ + "salary FLOAT, "
+ + "dept VARCHAR(32))");
+
+ st.executeUpdate("INSERT INTO " + tableName + " VALUES("
+ + "2,'Aaron','2009-05-14',1000000.00,'engineering')");
+ connection.commit();
+ } finally {
+ if (null != st) {
+ st.close();
+ }
+
+ if (null != connection) {
+ connection.close();
+ }
+ }
+
+ String [] expectedResults = {
+ "2,Aaron,2009-05-14,1000000.0,engineering"
+ };
+
+ doImport(false, false, tableName, expectedResults, null);
}
}
Modified: hadoop/mapreduce/trunk/src/contrib/sqoop/src/test/org/apache/hadoop/sqoop/manager/PostgresqlTest.java
URL: http://svn.apache.org/viewvc/hadoop/mapreduce/trunk/src/contrib/sqoop/src/test/org/apache/hadoop/sqoop/manager/PostgresqlTest.java?rev=891920&r1=891919&r2=891920&view=diff
==============================================================================
--- hadoop/mapreduce/trunk/src/contrib/sqoop/src/test/org/apache/hadoop/sqoop/manager/PostgresqlTest.java (original)
+++ hadoop/mapreduce/trunk/src/contrib/sqoop/src/test/org/apache/hadoop/sqoop/manager/PostgresqlTest.java Thu Dec 17 21:31:08 2009
@@ -36,6 +36,7 @@
import org.apache.hadoop.fs.Path;
import org.apache.hadoop.io.IOUtils;
import org.apache.hadoop.sqoop.SqoopOptions;
+import org.apache.hadoop.sqoop.testutil.CommonArgs;
import org.apache.hadoop.sqoop.testutil.ImportJobTestCase;
import org.apache.hadoop.sqoop.util.FileListing;
@@ -150,8 +151,7 @@
private String [] getArgv(boolean isDirect) {
ArrayList<String> args = new ArrayList<String>();
- args.add("-D");
- args.add("fs.default.name=file:///");
+ CommonArgs.addHadoopFlags(args);
args.add("--table");
args.add(TABLE_NAME);