Skip to content

[SPARK-52288][PS] Avoid INVALID_ARRAY_INDEX in split/rsplit when ANSI mode is on #51006

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.

Sign up for GitHub

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

Closed
wants to merge 3 commits into from
Closed

[SPARK-52288][PS] Avoid INVALID_ARRAY_INDEX in split/rsplit when ANSI mode is on #51006

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b1be33d
fix + test
xinrong-meng May 24, 2025
2e0e41e
rstring
xinrong-meng May 28, 2025
3fc1656
utility
xinrong-meng Jun 2, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 15 additions & 2 deletions python/pyspark/pandas/strings.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
import numpy as np
import pandas as pd

from pyspark.pandas.utils import is_ansi_mode_enabled
from pyspark.sql.types import StringType, BinaryType, ArrayType, LongType, MapType
from pyspark.sql import functions as F
from pyspark.sql.functions import pandas_udf
Expand Down Expand Up @@ -2031,7 +2032,13 @@ def pudf(s: pd.Series) -> pd.Series:
if expand:
psdf = psser.to_frame()
scol = psdf._internal.data_spark_columns[0]
spark_columns = [scol[i].alias(str(i)) for i in range(n + 1)]
spark_session = self._data._internal.spark_frame.sparkSession
if is_ansi_mode_enabled(spark_session):
spark_columns = [
F.try_element_at(scol, F.lit(i + 1)).alias(str(i)) for i in range(n + 1)
Copy link

@abhiips07 abhiips07 May 26, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

can we try to calculate f.lit(i+1) outside loop? since this might avoid function calls and object creation during loop execution

literals = [F.lit(i + 1) for i in range(n + 1)]

spark_columns = [F.try_element_at(scol, lit).alias(str(i)) for i, lit in enumerate(literals)]

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for suggestion!
There might not be a significant perf difference between creating F.lit inside the loop or beforehand, it's just wrapping a Python literal into a Spark expression, which aren’t executed immediately(just nodes in the DAG), and will be deduplicated by Catalyst. With that being said I’d like to keep the original for simplicity, but feel free to share if you have other opinions!

]
else:
spark_columns = [scol[i].alias(str(i)) for i in range(n + 1)]
column_labels = [(i,) for i in range(n + 1)]
internal = psdf._internal.with_new_columns(
spark_columns,
Expand Down Expand Up @@ -2178,7 +2185,13 @@ def pudf(s: pd.Series) -> pd.Series:
if expand:
psdf = psser.to_frame()
scol = psdf._internal.data_spark_columns[0]
spark_columns = [scol[i].alias(str(i)) for i in range(n + 1)]
spark_session = self._data._internal.spark_frame.sparkSession
if is_ansi_mode_enabled(spark_session):
spark_columns = [
F.try_element_at(scol, F.lit(i + 1)).alias(str(i)) for i in range(n + 1)
]
else:
spark_columns = [scol[i].alias(str(i)) for i in range(n + 1)]
column_labels = [(i,) for i in range(n + 1)]
internal = psdf._internal.with_new_columns(
spark_columns,
Expand Down
7 changes: 4 additions & 3 deletions python/pyspark/pandas/tests/series/test_string_ops_adv.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
from pyspark import pandas as ps
from pyspark.testing.pandasutils import PandasOnSparkTestCase
from pyspark.testing.sqlutils import SQLTestUtils
from pyspark.testing.utils import is_ansi_mode_test, ansi_mode_not_supported_message


class SeriesStringOpsAdvMixin:
Expand Down Expand Up @@ -174,7 +173,6 @@ def test_string_slice_replace(self):
self.check_func(lambda x: x.str.slice_replace(stop=2, repl="X"))
self.check_func(lambda x: x.str.slice_replace(start=1, stop=3, repl="X"))

@unittest.skipIf(is_ansi_mode_test, ansi_mode_not_supported_message)
def test_string_split(self):
self.check_func_on_series(lambda x: repr(x.str.split()), self.pser[:-1])
self.check_func_on_series(lambda x: repr(x.str.split(r"p*")), self.pser[:-1])
Expand All @@ -185,7 +183,8 @@ def test_string_split(self):
with self.assertRaises(NotImplementedError):
self.check_func(lambda x: x.str.split(expand=True))

@unittest.skipIf(is_ansi_mode_test, ansi_mode_not_supported_message)
self.check_func_on_series(lambda x: repr(x.str.split("-", n=1, expand=True)), pser)

def test_string_rsplit(self):
self.check_func_on_series(lambda x: repr(x.str.rsplit()), self.pser[:-1])
self.check_func_on_series(lambda x: repr(x.str.rsplit(r"p*")), self.pser[:-1])
Expand All @@ -196,6 +195,8 @@ def test_string_rsplit(self):
with self.assertRaises(NotImplementedError):
self.check_func(lambda x: x.str.rsplit(expand=True))

self.check_func_on_series(lambda x: repr(x.str.rsplit("-", n=1, expand=True)), pser)

def test_string_translate(self):
m = str.maketrans({"a": "X", "e": "Y", "i": None})
self.check_func(lambda x: x.str.translate(m))
Expand Down