[SPARK-28178][SQL] DataSourceV2: DataFrameWriter.insertInfo

[jzhuge]

What changes were proposed in this pull request?

Support multiple catalogs in the following InsertInto use cases:

• DataFrameWriter.insertInto("catalog.db.tbl")

Support matrix:

SaveMode	Partitioned Table	Partition Overwrite Mode	Action
Append	*	*	AppendData
Overwrite	no	*	OverwriteByExpression(true)
Overwrite	yes	STATIC	OverwriteByExpression(true)
Overwrite	yes	DYNAMIC	OverwritePartitionsDynamic

How was this patch tested?

New tests.
All existing catalyst and sql/core tests.

[SparkQA]

Test build #106958 has finished for PR 24980 at commit 9de941e.

• This patch passes all tests.
• This patch merges cleanly.
• This patch adds no public classes.

[SparkQA]

Test build #106971 has finished for PR 24980 at commit 34cd710.

• This patch fails Spark unit tests.
• This patch merges cleanly.
• This patch adds no public classes.

[SparkQA]

Test build #106972 has finished for PR 24980 at commit eaf2336.

• This patch passes all tests.
• This patch merges cleanly.
• This patch adds no public classes.

[SparkQA]

Test build #107140 has finished for PR 24980 at commit fb45cb1.

• This patch fails SparkR unit tests.
• This patch merges cleanly.
• This patch adds the following public classes (experimental):
• class KafkaTable extends Table with SupportsRead with SupportsWrite
• public final class ColumnarBatch implements AutoCloseable
• case class DummyExpressionHolder(exprs: Seq[Expression]) extends LeafNode
• abstract class QuaternaryExpression extends Expression
• case class CheckOverflow(
• case class Overlay(input: Expression, replace: Expression, pos: Expression, len: Expression)
• case class MapPartitionsInPandas(
• class ColumnarRule
• case class ColumnarToRowExec(child: SparkPlan)
• case class RowToColumnarExec(child: SparkPlan) extends UnaryExecNode
• case class ApplyColumnarRulesAndInsertTransitions(conf: SQLConf, columnarRules: Seq[ColumnarRule])
• case class InputAdapter(child: SparkPlan, isChildColumnar: Boolean)
• case class MapPartitionsInPandasExec(

[SparkQA]

Test build #107143 has finished for PR 24980 at commit 3c3aa04.

• This patch passes all tests.
• This patch merges cleanly.
• This patch adds no public classes.

[dongjoon-hyun]

Retest this please.

[SparkQA]

Test build #107313 has finished for PR 24980 at commit 3c3aa04.

• This patch fails Spark unit tests.
• This patch merges cleanly.
• This patch adds no public classes.

[SparkQA]

Test build #108235 has finished for PR 24980 at commit 65a0d43.

• This patch passes all tests.
• This patch merges cleanly.
• This patch adds no public classes.

[jzhuge]

@dongjoon-hyun@brkyvz@cloud-fan@rdblue This PR is ready for review. It is a follow-up to DSv2 INSERT INTO.

[cloud-fan]

shall we move these 2 checks to the public insertTo method, instead of duplicating it in the 2 private methods?

[cloud-fan]

what do you mean by dynamic clause?

[jzhuge]

This is a copy-paste issue. I will remove " - dynamic clause" from the title. insertInto does not have anything similar to INSERT INTO's PARTITION clause.

[SparkQA]

Test build #108338 has finished for PR 24980 at commit c4eeee5.

• This patch passes all tests.
• This patch merges cleanly.
• This patch adds no public classes.

[cloud-fan]

thanks, merging to master!

[jzhuge]

Thanks @cloud-fan !

[cloud-fan]

I missed it. If you look at the doc of insertInto, it says

 * Inserts the content of the `DataFrame` to the specified table. It requires that
* the schema of the `DataFrame` is the same as the schema of the table.
*
* @note Unlike `saveAsTable`, `insertInto` ignores the column names and just uses position-based
* resolution. For example:

We should use byPosition here.

[rdblue]

Agreed. This is an oversight and should be by position.

[jzhuge]

Thanks @cloud-fan@rdblue. I will submit a hotfix.

[jzhuge]

I can create a follow-up PR to introduce an option matchByName, default to false. If true, insertInto uses byName; otherwise, byPosition.

Maybe even included in this PR?

[cloud-fan]

If we are going to create a new dataframe writer API in the future, I'd like to keep it as it is, and always do by-position in this insertInto.

[rdblue]

If we are going to create a new dataframe writer API in the future, I'd like to keep it as it is, and always do by-position in this insertInto.

Sounds good to me. I'll submit a PR for the new API.

[jzhuge]

@rdblue@cloud-fan@dongjoon-hyun#25353 is ready for review.