Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SPARK-42876][SQL] DataType's physicalDataType should be private[sql] #40499

Closed

Conversation

amaliujia
Copy link
Contributor

What changes were proposed in this pull request?

physicalDataType should not be a public API but be private[sql].

Why are the changes needed?

This is to limit API scope to not expose unnecessary API to be public.

Does this PR introduce any user-facing change?

No since we have not released Spark 3.4.0 yet.

How was this patch tested?

N/A

@github-actions github-actions bot added the SQL label Mar 21, 2023
@@ -119,7 +119,7 @@ abstract class DataType extends AbstractDataType {

override private[sql] def acceptsType(other: DataType): Boolean = sameType(other)

def physicalDataType: PhysicalDataType = UninitializedPhysicalType
private[sql] def physicalDataType: PhysicalDataType = UninitializedPhysicalType
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we change the modifier in subclasses as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@cloud-fan
Copy link
Contributor

thanks, merging to master/3.4!

@cloud-fan cloud-fan closed this in c9a530e Mar 21, 2023
cloud-fan pushed a commit that referenced this pull request Mar 21, 2023
### What changes were proposed in this pull request?

`physicalDataType` should not be a public API but be private[sql].

### Why are the changes needed?

This is to limit API scope to not expose unnecessary API to be public.

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

No since we have not released Spark 3.4.0 yet.

### How was this patch tested?

N/A

Closes #40499 from amaliujia/change_scope_of_physical_data_type.

Authored-by: Rui Wang <rui.wang@databricks.com>
Signed-off-by: Wenchen Fan <wenchen@databricks.com>
(cherry picked from commit c9a530e)
Signed-off-by: Wenchen Fan <wenchen@databricks.com>
@cloud-fan
Copy link
Contributor

also cc @xinrong-meng this is from api auditing

yaooqinn pushed a commit that referenced this pull request Mar 24, 2023
…d generating API doc

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

This is the only issue I found during SQL module API auditing via apache/spark-website@6159860 . Somehow `protected[sql]` also generates API doc which is unexpected. `private[sql]` solves the problem and I generated doc locally to verify it.

Another API issue has been fixed by #40499

### Why are the changes needed?

fix api doc

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

no

### How was this patch tested?

N/A

Closes #40541 from cloud-fan/auditing.

Authored-by: Wenchen Fan <wenchen@databricks.com>
Signed-off-by: Kent Yao <yao@apache.org>
yaooqinn pushed a commit that referenced this pull request Mar 24, 2023
…d generating API doc

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

This is the only issue I found during SQL module API auditing via apache/spark-website@6159860 . Somehow `protected[sql]` also generates API doc which is unexpected. `private[sql]` solves the problem and I generated doc locally to verify it.

Another API issue has been fixed by #40499

### Why are the changes needed?

fix api doc

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

no

### How was this patch tested?

N/A

Closes #40541 from cloud-fan/auditing.

Authored-by: Wenchen Fan <wenchen@databricks.com>
Signed-off-by: Kent Yao <yao@apache.org>
(cherry picked from commit f7421b4)
Signed-off-by: Kent Yao <yao@apache.org>
snmvaughan pushed a commit to snmvaughan/spark that referenced this pull request Jun 20, 2023
### What changes were proposed in this pull request?

`physicalDataType` should not be a public API but be private[sql].

### Why are the changes needed?

This is to limit API scope to not expose unnecessary API to be public.

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

No since we have not released Spark 3.4.0 yet.

### How was this patch tested?

N/A

Closes apache#40499 from amaliujia/change_scope_of_physical_data_type.

Authored-by: Rui Wang <rui.wang@databricks.com>
Signed-off-by: Wenchen Fan <wenchen@databricks.com>
(cherry picked from commit c9a530e)
Signed-off-by: Wenchen Fan <wenchen@databricks.com>
snmvaughan pushed a commit to snmvaughan/spark that referenced this pull request Jun 20, 2023
…d generating API doc

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

This is the only issue I found during SQL module API auditing via apache/spark-website@6159860 . Somehow `protected[sql]` also generates API doc which is unexpected. `private[sql]` solves the problem and I generated doc locally to verify it.

Another API issue has been fixed by apache#40499

### Why are the changes needed?

fix api doc

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

no

### How was this patch tested?

N/A

Closes apache#40541 from cloud-fan/auditing.

Authored-by: Wenchen Fan <wenchen@databricks.com>
Signed-off-by: Kent Yao <yao@apache.org>
(cherry picked from commit f7421b4)
Signed-off-by: Kent Yao <yao@apache.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants