[GitHub] spark pull request #15423: [SPARK-17860][SQL] SHOW COLUMN's database conflic...

[asfgit]

Github user asfgit closed the pull request at:

https://github.com/apache/spark/pull/15423


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request #15423: [SPARK-17860][SQL] SHOW COLUMN's database conflic...

[cloud-fan]

Github user cloud-fan commented on a diff in the pull request:

https://github.com/apache/spark/pull/15423#discussion_r84204534
  
--- Diff: 
sql/core/src/test/scala/org/apache/spark/sql/execution/command/DDLSuite.scala 
---
@@ -1713,4 +1713,21 @@ class DDLSuite extends QueryTest with 
SharedSQLContext with BeforeAndAfterEach {
   assert(sql("show user functions").count() === 1L)
 }
   }
+
+  test("show columns - negative test") {
+// When case sensitivity is true, the user supplied database name in 
table identifier
+// should match the supplied database name in case sensitive way.
+withSQLConf(SQLConf.CASE_SENSITIVE.key -> "true") {
+  withTempDatabase { db =>
+val tabName = s"$db.showcolumn"
+withTable(tabName) {
+  sql(s"CREATE TABLE $tabName(col1 int, col2 string) USING parquet 
")
+  val message = intercept[AnalysisException] {
+  sql(s"SHOW COLUMNS IN $db.showcolumn FROM ${db.toUpperCase}")
--- End diff --

nit: wrong ident here.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request #15423: [SPARK-17860][SQL] SHOW COLUMN's database conflic...

[cloud-fan]

Github user cloud-fan commented on a diff in the pull request:

https://github.com/apache/spark/pull/15423#discussion_r8378
  
--- Diff: 
sql/core/src/main/scala/org/apache/spark/sql/execution/command/tables.scala ---
@@ -651,14 +651,28 @@ case class ShowTablePropertiesCommand(table: 
TableIdentifier, propertyKey: Optio
  *   SHOW COLUMNS (FROM | IN) table_identifier [(FROM | IN) database];
  * }}}
  */
-case class ShowColumnsCommand(tableName: TableIdentifier) extends 
RunnableCommand {
+case class ShowColumnsCommand(
+databaseName: Option[String],
+tableName: TableIdentifier) extends RunnableCommand {
   override val output: Seq[Attribute] = {
 AttributeReference("col_name", StringType, nullable = false)() :: Nil
   }
 
+  private def nameEqual(name1: String, name2: String, caseSensitive: 
Boolean): Boolean = {
+if (caseSensitive) name1 == name2 else name1.equalsIgnoreCase(name2)
+  }
+
   override def run(sparkSession: SparkSession): Seq[Row] = {
 val catalog = sparkSession.sessionState.catalog
-val table = catalog.getTempViewOrPermanentTableMetadata(tableName)
+val caseSensitive = 
sparkSession.sessionState.conf.caseSensitiveAnalysis
--- End diff --

nit: we can simplify it to
```
val resolver = sparkSession.sessionState.conf.resolver
...
case Some(db) if tableName.database.exists(!resolver(_, db))
```


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request #15423: [SPARK-17860][SQL] SHOW COLUMN's database conflic...

[viirya]

Github user viirya commented on a diff in the pull request:

https://github.com/apache/spark/pull/15423#discussion_r83771158
  
--- Diff: 
sql/core/src/main/scala/org/apache/spark/sql/execution/SparkSqlParser.scala ---
@@ -168,17 +168,7 @@ class SparkSqlAstBuilder(conf: SQLConf) extends 
AstBuilder {
* }}}
*/
   override def visitShowColumns(ctx: ShowColumnsContext): LogicalPlan = 
withOrigin(ctx) {
-val table = visitTableIdentifier(ctx.tableIdentifier)
-
-val lookupTable = Option(ctx.db) match {
-  case None => table
-  case Some(db) if table.database.exists(_ != db) =>
-operationNotAllowed(
-  s"SHOW COLUMNS with conflicting databases: '$db' != 
'${table.database.get}'",
-  ctx)
-  case Some(db) => TableIdentifier(table.identifier, Some(db.getText))
-}
-ShowColumnsCommand(lookupTable)
+ShowColumnsCommand(Option(ctx.db).map(_.getText), 
visitTableIdentifier(ctx.tableIdentifier))
--- End diff --

I have no strong option towards this. From my point, MySQL's way might be 
little confusing users if they don't notice the database name is different.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request #15423: [SPARK-17860][SQL] SHOW COLUMN's database conflic...

[dilipbiswal]

Github user dilipbiswal commented on a diff in the pull request:

https://github.com/apache/spark/pull/15423#discussion_r82938410
  
--- Diff: 
sql/core/src/test/scala/org/apache/spark/sql/SQLQueryTestSuite.scala ---
@@ -207,6 +208,7 @@ class SQLQueryTestSuite extends QueryTest with 
SharedSQLContext {
 // Returns true if the plan is supposed to be sorted.
 def isSorted(plan: LogicalPlan): Boolean = plan match {
   case _: Join | _: Aggregate | _: Generate | _: Sample | _: Distinct 
=> false
+  case _: ShowColumnsCommand => true
--- End diff --

@cloud-fan @viirya Thanks :-) I will change it.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request #15423: [SPARK-17860][SQL] SHOW COLUMN's database conflic...

[viirya]

Github user viirya commented on a diff in the pull request:

https://github.com/apache/spark/pull/15423#discussion_r82937473
  
--- Diff: 
sql/core/src/test/scala/org/apache/spark/sql/SQLQueryTestSuite.scala ---
@@ -207,6 +208,7 @@ class SQLQueryTestSuite extends QueryTest with 
SharedSQLContext {
 // Returns true if the plan is supposed to be sorted.
 def isSorted(plan: LogicalPlan): Boolean = plan match {
   case _: Join | _: Aggregate | _: Generate | _: Sample | _: Distinct 
=> false
+  case _: ShowColumnsCommand => true
--- End diff --

+1 as mentioned in previous comment.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request #15423: [SPARK-17860][SQL] SHOW COLUMN's database conflic...

[cloud-fan]

Github user cloud-fan commented on a diff in the pull request:

https://github.com/apache/spark/pull/15423#discussion_r82937255
  
--- Diff: 
sql/core/src/test/scala/org/apache/spark/sql/SQLQueryTestSuite.scala ---
@@ -207,6 +208,7 @@ class SQLQueryTestSuite extends QueryTest with 
SharedSQLContext {
 // Returns true if the plan is supposed to be sorted.
 def isSorted(plan: LogicalPlan): Boolean = plan match {
   case _: Join | _: Aggregate | _: Generate | _: Sample | _: Distinct 
=> false
+  case _: ShowColumnsCommand => true
--- End diff --

marking `ShowColumnsCommand` as sorted is more weird, I'd like to leave the 
result sorted.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request #15423: [SPARK-17860][SQL] SHOW COLUMN's database conflic...

[dilipbiswal]

Github user dilipbiswal commented on a diff in the pull request:

https://github.com/apache/spark/pull/15423#discussion_r82818691
  
--- Diff: 
sql/core/src/main/scala/org/apache/spark/sql/execution/SparkSqlParser.scala ---
@@ -168,17 +168,7 @@ class SparkSqlAstBuilder(conf: SQLConf) extends 
AstBuilder {
* }}}
*/
   override def visitShowColumns(ctx: ShowColumnsContext): LogicalPlan = 
withOrigin(ctx) {
-val table = visitTableIdentifier(ctx.tableIdentifier)
-
-val lookupTable = Option(ctx.db) match {
-  case None => table
-  case Some(db) if table.database.exists(_ != db) =>
-operationNotAllowed(
-  s"SHOW COLUMNS with conflicting databases: '$db' != 
'${table.database.get}'",
-  ctx)
-  case Some(db) => TableIdentifier(table.identifier, Some(db.getText))
-}
-ShowColumnsCommand(lookupTable)
+ShowColumnsCommand(Option(ctx.db).map(_.getText), 
visitTableIdentifier(ctx.tableIdentifier))
--- End diff --

Thanks @viirya for checking hive. Yeah, when we implemented the SHOW 
columns native command, wde went through the the above hive code. We decided to 
improve up on the above check and report an error only when the database names 
are different. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request #15423: [SPARK-17860][SQL] SHOW COLUMN's database conflic...

[dilipbiswal]

Github user dilipbiswal commented on a diff in the pull request:

https://github.com/apache/spark/pull/15423#discussion_r82817760
  
--- Diff: 
sql/core/src/test/scala/org/apache/spark/sql/SQLQueryTestSuite.scala ---
@@ -207,6 +208,7 @@ class SQLQueryTestSuite extends QueryTest with 
SharedSQLContext {
 // Returns true if the plan is supposed to be sorted.
 def isSorted(plan: LogicalPlan): Boolean = plan match {
   case _: Join | _: Aggregate | _: Generate | _: Sample | _: Distinct 
=> false
+  case _: ShowColumnsCommand => true
--- End diff --

@cloud-fan @viirya Actually it does not break the test if we don't mark it 
as sorted. What happens is that, when we generate the expected output file, the 
results appear sorted like following:

```SQL
-- !query 7
SHOW COLUMNS IN showcolumn2 IN showdb
-- !query 7 schema
struct
-- !query 7 output
month
price
qty
year
```
When i was going through the expected output file to make sure its correct, 
i noticed this as the above output would not be how it would be shown if i 
cut-paste the SQLs snippets from the test file and ran it in spark-sql shell. 

If you guys think its okay to have the output in sorted form in the 
expected file, then i will change it back.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request #15423: [SPARK-17860][SQL] SHOW COLUMN's database conflic...

[cloud-fan]

Github user cloud-fan commented on a diff in the pull request:

https://github.com/apache/spark/pull/15423#discussion_r82797867
  
--- Diff: 
sql/core/src/test/scala/org/apache/spark/sql/SQLQueryTestSuite.scala ---
@@ -207,6 +208,7 @@ class SQLQueryTestSuite extends QueryTest with 
SharedSQLContext {
 // Returns true if the plan is supposed to be sorted.
 def isSorted(plan: LogicalPlan): Boolean = plan match {
   case _: Join | _: Aggregate | _: Generate | _: Sample | _: Distinct 
=> false
+  case _: ShowColumnsCommand => true
--- End diff --

Does it break test if we don't mark it as sorted?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request #15423: [SPARK-17860][SQL] SHOW COLUMN's database conflic...

[viirya]

Github user viirya commented on a diff in the pull request:

https://github.com/apache/spark/pull/15423#discussion_r82750631
  
--- Diff: 
sql/core/src/main/scala/org/apache/spark/sql/execution/SparkSqlParser.scala ---
@@ -168,17 +168,7 @@ class SparkSqlAstBuilder(conf: SQLConf) extends 
AstBuilder {
* }}}
*/
   override def visitShowColumns(ctx: ShowColumnsContext): LogicalPlan = 
withOrigin(ctx) {
-val table = visitTableIdentifier(ctx.tableIdentifier)
-
-val lookupTable = Option(ctx.db) match {
-  case None => table
-  case Some(db) if table.database.exists(_ != db) =>
-operationNotAllowed(
-  s"SHOW COLUMNS with conflicting databases: '$db' != 
'${table.database.get}'",
-  ctx)
-  case Some(db) => TableIdentifier(table.identifier, Some(db.getText))
-}
-ShowColumnsCommand(lookupTable)
+ShowColumnsCommand(Option(ctx.db).map(_.getText), 
visitTableIdentifier(ctx.tableIdentifier))
--- End diff --

Seems Hive doesn't allow specifying duplicate databases no matter they are 
the same or not.


https://github.com/apache/hive/blob/21a0142f333fba231f2648db53a48dc41384ad72/ql/src/java/org/apache/hadoop/hive/ql/parse/DDLSemanticAnalyzer.java#L2215


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request #15423: [SPARK-17860][SQL] SHOW COLUMN's database conflic...

[viirya]

Github user viirya commented on a diff in the pull request:

https://github.com/apache/spark/pull/15423#discussion_r82750409
  
--- Diff: 
sql/core/src/main/scala/org/apache/spark/sql/execution/SparkSqlParser.scala ---
@@ -168,17 +168,7 @@ class SparkSqlAstBuilder(conf: SQLConf) extends 
AstBuilder {
* }}}
*/
   override def visitShowColumns(ctx: ShowColumnsContext): LogicalPlan = 
withOrigin(ctx) {
-val table = visitTableIdentifier(ctx.tableIdentifier)
-
-val lookupTable = Option(ctx.db) match {
-  case None => table
-  case Some(db) if table.database.exists(_ != db) =>
-operationNotAllowed(
-  s"SHOW COLUMNS with conflicting databases: '$db' != 
'${table.database.get}'",
-  ctx)
-  case Some(db) => TableIdentifier(table.identifier, Some(db.getText))
-}
-ShowColumnsCommand(lookupTable)
+ShowColumnsCommand(Option(ctx.db).map(_.getText), 
visitTableIdentifier(ctx.tableIdentifier))
--- End diff --

BTW, what @dilipbiswal does in this is following previous behavior, do we 
want to break it?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request #15423: [SPARK-17860][SQL] SHOW COLUMN's database conflic...

[viirya]

Github user viirya commented on a diff in the pull request:

https://github.com/apache/spark/pull/15423#discussion_r82745403
  
--- Diff: 
sql/core/src/main/scala/org/apache/spark/sql/execution/SparkSqlParser.scala ---
@@ -168,17 +168,7 @@ class SparkSqlAstBuilder(conf: SQLConf) extends 
AstBuilder {
* }}}
*/
   override def visitShowColumns(ctx: ShowColumnsContext): LogicalPlan = 
withOrigin(ctx) {
-val table = visitTableIdentifier(ctx.tableIdentifier)
-
-val lookupTable = Option(ctx.db) match {
-  case None => table
-  case Some(db) if table.database.exists(_ != db) =>
-operationNotAllowed(
-  s"SHOW COLUMNS with conflicting databases: '$db' != 
'${table.database.get}'",
-  ctx)
-  case Some(db) => TableIdentifier(table.identifier, Some(db.getText))
-}
-ShowColumnsCommand(lookupTable)
+ShowColumnsCommand(Option(ctx.db).map(_.getText), 
visitTableIdentifier(ctx.tableIdentifier))
--- End diff --

Good  point!


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request #15423: [SPARK-17860][SQL] SHOW COLUMN's database conflic...

[cloud-fan]

Github user cloud-fan commented on a diff in the pull request:

https://github.com/apache/spark/pull/15423#discussion_r82744230
  
--- Diff: 
sql/core/src/main/scala/org/apache/spark/sql/execution/SparkSqlParser.scala ---
@@ -168,17 +168,7 @@ class SparkSqlAstBuilder(conf: SQLConf) extends 
AstBuilder {
* }}}
*/
   override def visitShowColumns(ctx: ShowColumnsContext): LogicalPlan = 
withOrigin(ctx) {
-val table = visitTableIdentifier(ctx.tableIdentifier)
-
-val lookupTable = Option(ctx.db) match {
-  case None => table
-  case Some(db) if table.database.exists(_ != db) =>
-operationNotAllowed(
-  s"SHOW COLUMNS with conflicting databases: '$db' != 
'${table.database.get}'",
-  ctx)
-  case Some(db) => TableIdentifier(table.identifier, Some(db.getText))
-}
-ShowColumnsCommand(lookupTable)
+ShowColumnsCommand(Option(ctx.db).map(_.getText), 
visitTableIdentifier(ctx.tableIdentifier))
--- End diff --

FYI, MySQL will treat `SHOW COLUMNS FROM db1.tbl1 FROM db2` as `SHOW 
COLUMNS FROM tbl1 FROM db2`, i.e. if `FROM database` is specified, it will just 
ignore the database specified in table name, instead of reporting error.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request #15423: [SPARK-17860][SQL] SHOW COLUMN's database conflic...

[dilipbiswal]

Github user dilipbiswal commented on a diff in the pull request:

https://github.com/apache/spark/pull/15423#discussion_r82721435
  
--- Diff: 
sql/core/src/test/scala/org/apache/spark/sql/execution/command/DDLSuite.scala 
---
@@ -1713,4 +1713,19 @@ class DDLSuite extends QueryTest with 
SharedSQLContext with BeforeAndAfterEach {
   assert(sql("show user functions").count() === 1L)
 }
   }
+
+  test("show columns - negative test") {
+// When case sensitivity is true, the user supplied database name in 
table identifier
+// should match the supplied database name in case sensitive way.
+withSQLConf(SQLConf.CASE_SENSITIVE.key -> "true") {
+  val tabName = "showcolumn"
+  withTable(tabName) {
+sql(s"CREATE TABLE $tabName(col1 int, col2 string) USING parquet ")
--- End diff --

@viirya OK.. I agree. I will make the change


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request #15423: [SPARK-17860][SQL] SHOW COLUMN's database conflic...

[viirya]

Github user viirya commented on a diff in the pull request:

https://github.com/apache/spark/pull/15423#discussion_r82720911
  
--- Diff: 
sql/core/src/test/scala/org/apache/spark/sql/SQLQueryTestSuite.scala ---
@@ -207,6 +208,7 @@ class SQLQueryTestSuite extends QueryTest with 
SharedSQLContext {
 // Returns true if the plan is supposed to be sorted.
 def isSorted(plan: LogicalPlan): Boolean = plan match {
   case _: Join | _: Aggregate | _: Generate | _: Sample | _: Distinct 
=> false
+  case _: ShowColumnsCommand => true
--- End diff --

Personally I don't think it is odd because we just want to compare the 
results. Adding `ShowColumnsCommand` to sorted op looks more odd to me. cc 
@cloud-fan 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request #15423: [SPARK-17860][SQL] SHOW COLUMN's database conflic...

[dilipbiswal]

Github user dilipbiswal commented on a diff in the pull request:

https://github.com/apache/spark/pull/15423#discussion_r82720521
  
--- Diff: 
sql/core/src/test/scala/org/apache/spark/sql/SQLQueryTestSuite.scala ---
@@ -207,6 +208,7 @@ class SQLQueryTestSuite extends QueryTest with 
SharedSQLContext {
 // Returns true if the plan is supposed to be sorted.
 def isSorted(plan: LogicalPlan): Boolean = plan match {
   case _: Join | _: Aggregate | _: Generate | _: Sample | _: Distinct 
=> false
+  case _: ShowColumnsCommand => true
--- End diff --

@viirya So it seemed odd to have the generated output files to have column 
names sorted which didn't reflect the columns. In the test case i had the table 
create like following.
```SQL
CREATE TABLE showcolumn2 (price int, qty int) partitioned by (year int, 
month int)
```
It seemed odd to me to have the generated output file report the columns as 
month, price, qty and year as opposed to price, qty, year and month.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request #15423: [SPARK-17860][SQL] SHOW COLUMN's database conflic...

[viirya]

Github user viirya commented on a diff in the pull request:

https://github.com/apache/spark/pull/15423#discussion_r82718263
  
--- Diff: 
sql/core/src/test/scala/org/apache/spark/sql/SQLQueryTestSuite.scala ---
@@ -207,6 +208,7 @@ class SQLQueryTestSuite extends QueryTest with 
SharedSQLContext {
 // Returns true if the plan is supposed to be sorted.
 def isSorted(plan: LogicalPlan): Boolean = plan match {
   case _: Join | _: Aggregate | _: Generate | _: Sample | _: Distinct 
=> false
+  case _: ShowColumnsCommand => true
--- End diff --

Will it affect comparison result? I think the result is generated, right?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request #15423: [SPARK-17860][SQL] SHOW COLUMN's database conflic...

[dilipbiswal]

Github user dilipbiswal commented on a diff in the pull request:

https://github.com/apache/spark/pull/15423#discussion_r82717035
  
--- Diff: 
sql/core/src/test/scala/org/apache/spark/sql/SQLQueryTestSuite.scala ---
@@ -207,6 +208,7 @@ class SQLQueryTestSuite extends QueryTest with 
SharedSQLContext {
 // Returns true if the plan is supposed to be sorted.
 def isSorted(plan: LogicalPlan): Boolean = plan match {
   case _: Join | _: Aggregate | _: Generate | _: Sample | _: Distinct 
=> false
+  case _: ShowColumnsCommand => true
--- End diff --

@viirya I added the case here because of

https://github.com/dilipbiswal/spark/blob/3acd08f9431d6cdfe4d043aa342d09fc0ebfa497/sql/core/src/test/scala/org/apache/spark/sql/SQLQueryTestSuite.scala#L222

I didn't want the output of ShowColumnsCommand to be sorted before 
comparison.



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request #15423: [SPARK-17860][SQL] SHOW COLUMN's database conflic...

[viirya]

Github user viirya commented on a diff in the pull request:

https://github.com/apache/spark/pull/15423#discussion_r82716067
  
--- Diff: 
sql/core/src/test/scala/org/apache/spark/sql/SQLQueryTestSuite.scala ---
@@ -207,6 +208,7 @@ class SQLQueryTestSuite extends QueryTest with 
SharedSQLContext {
 // Returns true if the plan is supposed to be sorted.
 def isSorted(plan: LogicalPlan): Boolean = plan match {
   case _: Join | _: Aggregate | _: Generate | _: Sample | _: Distinct 
=> false
+  case _: ShowColumnsCommand => true
--- End diff --

Why `ShowColumnsCommand` is sorted?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request #15423: [SPARK-17860][SQL] SHOW COLUMN's database conflic...

[dilipbiswal]

GitHub user dilipbiswal opened a pull request:

https://github.com/apache/spark/pull/15423

[SPARK-17860][SQL] SHOW COLUMN's database conflict check should respect 
case sensitivity configuration

## What changes were proposed in this pull request?
SHOW COLUMNS command validates the user supplied database 
name with database name from qualified table name name to make
sure both of them are consistent. This comparison should respect
case sensitivity.

## How was this patch tested?
Added tests in DDLSuite and existing tests were moved to use new sql based 
test infrastructure.


You can merge this pull request into a Git repository by running:

$ git pull https://github.com/dilipbiswal/spark dkb_show_column_fix

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/spark/pull/15423.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #15423


commit 3acd08f9431d6cdfe4d043aa342d09fc0ebfa497
Author: Dilip Biswal 
Date:   2016-09-29T04:37:40Z

SHOW COLUMN's database conflict check should respect case sensitivity 
configuration.




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org