Skip to content

Conversation

@itholic
Copy link
Contributor

@itholic itholic commented Apr 24, 2023

What changes were proposed in this pull request?

This is follow-up for #39991 to remove unused exception.

Why are the changes needed?

PySparkTypeError never raises because we checked the type already in the codes above if type(startPos) != type(length): and raise PySparkTypeError for unsupported type for length.

Does this PR introduce any user-facing change?

No. it's minor code cleanup

How was this patch tested?

The existing CI should pass.

@zhengruifeng
Copy link
Contributor

to be more consistent with

if isinstance(startPos, int):
jc = self._jc.substr(startPos, length)
elif isinstance(startPos, Column):
jc = self._jc.substr(startPos._jc, cast("Column", length)._jc)
else:
raise PySparkTypeError(
error_class="NOT_COLUMN_OR_INT",
message_parameters={"arg_name": "startPos", "arg_type": type(startPos).__name__},
)

what about changing to?

        if isinstance(length, Column):
            length_expr = length._expr
            start_expr = startPos._expr
        elif isinstance(length, int):
            length_expr = LiteralExpression._from_value(length)
            start_expr = LiteralExpression._from_value(startPos)
        else:
            raise PySparkTypeError(
                error_class="NOT_COLUMN_OR_INT",
                message_parameters={"arg_name": "length", "arg_type": type(length).__name__},
            )          

@itholic
Copy link
Contributor Author

itholic commented Apr 25, 2023

what about changing to?

@zhengruifeng Nice! Just applied the comment

@zhengruifeng
Copy link
Contributor

oh, the linter fails

@itholic
Copy link
Contributor Author

itholic commented Apr 25, 2023

Fixed!

@zhengruifeng
Copy link
Contributor

merged to master

@itholic itholic deleted the connect_col_followup branch April 27, 2023 10:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants