Skip to content

Conversation

@jiangxb1987
Copy link
Contributor

What changes were proposed in this pull request?

With the new approach of view resolution, we can get rid of SQL generation on view creation, so let's remove SQL builder for operators.

Note that, since all sql generation for operators is defined in one file (org.apache.spark.sql.catalyst.SQLBuilder), it’d be trivial to recover it in the future.

How was this patch tested?

N/A

@SparkQA
Copy link

SparkQA commented Feb 9, 2017

Test build #72636 has finished for PR 16869 at commit 52b4a5e.

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

@hvanhovell
Copy link
Contributor

cc @rxin @yhuai wdyt? Should we be bold and remove this?

@hvanhovell
Copy link
Contributor

LGTM. I'd like to wait a little with merging this in order to build some consensus.

@hvanhovell
Copy link
Contributor

I had an offline discussion with @rxin about this, and we have decided to merge this one.

@asfgit asfgit closed this in af63c52 Feb 9, 2017
@jiangxb1987 jiangxb1987 deleted the SQLBuilder branch February 10, 2017 01:29
@gatorsmile
Copy link
Member

Now, we can also remove the generated sqlgen files.

cmonkey pushed a commit to cmonkey/spark that referenced this pull request Feb 15, 2017
## What changes were proposed in this pull request?

With the new approach of view resolution, we can get rid of SQL generation on view creation, so let's remove SQL builder for operators.

Note that, since all sql generation for operators is defined in one file (org.apache.spark.sql.catalyst.SQLBuilder), it’d be trivial to recover it in the future.

## How was this patch tested?

N/A

Author: jiangxingbo <[email protected]>

Closes apache#16869 from jiangxb1987/SQLBuilder.
ghost pushed a commit to dbtsai/spark that referenced this pull request Aug 25, 2017
## What changes were proposed in this pull request?

After [SPARK-19025](apache#16869), there is no need to keep SQLBuilderTest.
ExpressionSQLBuilderSuite is the only place to use it.
This PR aims to remove SQLBuilderTest.

## How was this patch tested?

Pass the updated `ExpressionSQLBuilderSuite`.

Author: Dongjoon Hyun <[email protected]>

Closes apache#19044 from dongjoon-hyun/SPARK-21832.
ghost pushed a commit to dbtsai/spark that referenced this pull request Aug 26, 2017
…g in HiveCompatibilitySuite

## What changes were proposed in this pull request?

[SPARK-19025](apache#16869) removes SQLBuilder, so we don't need the following in HiveCompatibilitySuite.

```scala
// Ensures that the plans generation use metastore relation and not OrcRelation
// Was done because SqlBuilder does not work with plans having logical relation
TestHive.setConf(HiveUtils.CONVERT_METASTORE_ORC, false)
```

## How was this patch tested?

Pass the existing Jenkins tests.

Author: Dongjoon Hyun <[email protected]>

Closes apache#19043 from dongjoon-hyun/SPARK-21831.
@passionke
Copy link

@jiangxb1987

"With the new approach of view resolution, we can get rid of SQL generation on view creation, so let's remove SQL builder for operators."

can you introduce more abount "view resolution"? In my situation, I will use spark with logicalplan optimizer to build better logicalplan, and transfer this logicalplan back to sql for executing outside by spark, such as odps in alibaba etc.

so, can you give me some suggestion for my task? thanks very much!

@passionke
Copy link

@gatorsmile @ hvanhovell @rxin @ @yhuai

hvanhovell pushed a commit that referenced this pull request May 10, 2023
…QL`, `outputRowFormatSQL` method

### What changes were proposed in this pull request?

Remove useless code in class `ScriptInputOutputSchema`. Include `getRowFormatSQL`, `inputRowFormatSQL`, `outputRowFormatSQL` method. It unused when #16869

### Why are the changes needed?
Clear code.

### Does this PR introduce _any_ user-facing change?
No

### How was this patch tested?
Unnecessary.

Closes #41063 from Hisoka-X/remove_useless_code.

Authored-by: Hisoka <[email protected]>
Signed-off-by: Herman van Hovell <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants