Skip to content

Commit 2d3bc48

Browse files
justing-bqjusting-bq
committed
Address feedback
1 parent eeb4979 commit 2d3bc48

9 files changed

Lines changed: 61 additions & 34 deletions

File tree

‎.github/workflows/cpp_extra.yml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -515,7 +515,7 @@ jobs:
515515
uses: actions/upload-artifact@v7
516516
with:
517517
name: flight-sql-odbc-pkg-installer-${{ matrix.architecture }}
518-
path: build/cpp/ArrowFlightSqlOdbcODBC-*.pkg
518+
path: build/cpp/ArrowFlightSqlOdbc-*.pkg
519519
if-no-files-found: error
520520
- name: Register Flight SQL ODBC Driver
521521
run: |

‎cpp/src/arrow/flight/sql/odbc/CMakeLists.txt‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -161,7 +161,7 @@ if(ARROW_FLIGHT_SQL_ODBC_INSTALLER)
161161
else()
162162
if(APPLE)
163163
set(CPACK_PACKAGE_FILE_NAME
164-
"ArrowFlightSqlOdbcODBC-${CPACK_PACKAGE_VERSION_MAJOR}.${ODBC_PACKAGE_VERSION_MINOR}.${ODBC_PACKAGE_VERSION_PATCH}"
164+
"ArrowFlightSqlOdbc-${CPACK_PACKAGE_VERSION_MAJOR}.${ODBC_PACKAGE_VERSION_MINOR}.${ODBC_PACKAGE_VERSION_PATCH}"
165165
)
166166
set(CPACK_PACKAGE_INSTALL_DIRECTORY "${CPACK_PACKAGE_NAME}")
167167

@@ -172,10 +172,10 @@ if(ARROW_FLIGHT_SQL_ODBC_INSTALLER)
172172
"${CMAKE_CURRENT_SOURCE_DIR}/install/mac/postinstall")
173173
set(CPACK_RESOURCE_FILE_README "${CMAKE_CURRENT_SOURCE_DIR}/install/mac/README.txt")
174174
set(CPACK_RESOURCE_FILE_WELCOME
175-
"${CMAKE_CURRENT_SOURCE_DIR}/install/mac/Welcome.txt")
175+
"${CMAKE_CURRENT_SOURCE_DIR}/install/mac/welcome.txt")
176176

177177
set(ODBC_INSTALL_DIR "arrow-odbc/lib")
178-
set(DOC_INSTALL_DIR "arrow-odbc/doc")
178+
set(ODBC_DOC_INSTALL_DIR "arrow-odbc/doc")
179179
else()
180180
# Linux
181181
# GH-49595: TODO implement DEB installer
@@ -213,10 +213,10 @@ if(ARROW_FLIGHT_SQL_ODBC_INSTALLER)
213213

214214
# Install documentation files
215215
install(FILES "${CMAKE_CURRENT_SOURCE_DIR}/../../../../../../LICENSE.txt"
216-
DESTINATION "${DOC_INSTALL_DIR}"
216+
DESTINATION "${ODBC_DOC_INSTALL_DIR}"
217217
COMPONENT Docs)
218-
install(FILES "${CMAKE_CURRENT_SOURCE_DIR}/Connection-Options.md"
219-
DESTINATION "${DOC_INSTALL_DIR}"
218+
install(FILES "${CMAKE_CURRENT_SOURCE_DIR}/connection-options.md"
219+
DESTINATION "${ODBC_DOC_INSTALL_DIR}"
220220
COMPONENT Docs)
221221
endif()
222222

cpp/src/arrow/flight/sql/odbc/Connection-Options.md renamed to cpp/src/arrow/flight/sql/odbc/connection-options.md

File renamed without changes.

‎cpp/src/arrow/flight/sql/odbc/install/mac/README.txt‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
11
Files are available in '/Library/ODBC/arrow-odbc' after installation.
22

3-
To setup a connection, you can use DSN to store your data source connection information.
3+
To set up a connection, you can use DSN to store your data source connection information.
44
1. Open 'iODBC Data Source Administrator'.
55
2. To create a user DSN, go to 'User DSN' tab and click 'Add'.
66
3. Select 'Apache Arrow Flight SQL ODBC Driver' and click 'Finish'.
7-
4. Enter DSN name and connection string values.
8-
For the list of all supported options, check '/Library/ODBC/arrow-odbc/doc/Connection-Options.md'.
9-
5. Click 'Ok' to save the DSN.
7+
4. Enter DSN name and connection string values.
8+
For the list of all supported options, check '/Library/ODBC/arrow-odbc/doc/connection-options.md'.
9+
5. Click 'OK' to save the DSN.

‎cpp/src/arrow/flight/sql/odbc/install/mac/postinstall‎

Lines changed: 25 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -17,14 +17,32 @@
1717
# specific language governing permissions and limitations
1818
# under the License.
1919

20-
# Use temporary driver registration script to register ODBC driver in system DSN
21-
odbc_install_script="/Library/ODBC/arrow-odbc/lib/install_odbc.sh"
22-
"$odbc_install_script" /Library/ODBC/arrow-odbc/lib/libarrow_flight_sql_odbc.dylib
20+
set -euo pipefail
2321

24-
# Use temporary DSN registration script to register sample system DSN
22+
odbc_install_script="/Library/ODBC/arrow-odbc/lib/install_odbc.sh"
2523
dsn_install_script="/Library/ODBC/arrow-odbc/lib/install_odbc_ini.sh"
26-
"$dsn_install_script" /Library/ODBC/odbc.ini
24+
driver_lib="/Library/ODBC/arrow-odbc/lib/libarrow_flight_sql_odbc.dylib"
25+
dsn_file="/Library/ODBC/odbc.ini"
26+
27+
if [[ ! -x "$odbc_install_script" ]]; then
28+
echo "ERROR: ODBC install script $odbc_install_script not found" >&2
29+
exit 1
30+
fi
31+
32+
"$odbc_install_script" "$driver_lib" || {
33+
echo "ERROR: Failed to register ODBC driver ($driver_lib)" >&2
34+
exit 1
35+
}
36+
37+
if [[ ! -x "$dsn_install_script" ]]; then
38+
echo "ERROR: DSN install script $dsn_install_script not found" >&2
39+
exit 1
40+
fi
41+
42+
"$dsn_install_script" "$dsn_file" || {
43+
echo "ERROR: Failed to register DSN to ($dsn_file)" >&2
44+
exit 1
45+
}
2746

2847
# clean temporary script
29-
rm -f "$odbc_install_script"
30-
rm -f "$dsn_install_script"
48+
rm -f "$odbc_install_script" "$dsn_install_script"

cpp/src/arrow/flight/sql/odbc/install/mac/Welcome.txt renamed to cpp/src/arrow/flight/sql/odbc/install/mac/welcome.txt

File renamed without changes.

‎cpp/src/arrow/flight/sql/odbc/install/unix/install_odbc.sh‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -42,44 +42,44 @@ fi
4242

4343
case "$(uname)" in
4444
Linux)
45-
USER_ODBCINST_FILE="/etc/odbcinst.ini"
45+
ODBCINST_FILE="/etc/odbcinst.ini"
4646
;;
4747
*)
4848
# macOS
49-
USER_ODBCINST_FILE="/Library/ODBC/odbcinst.ini"
49+
ODBCINST_FILE="/Library/ODBC/odbcinst.ini"
5050
mkdir -p /Library/ODBC
5151
;;
5252
esac
5353

5454
DRIVER_NAME="Apache Arrow Flight SQL ODBC Driver"
5555

56-
touch "$USER_ODBCINST_FILE"
56+
touch "$ODBCINST_FILE"
5757

58-
if grep -q "^\[$DRIVER_NAME\]" "$USER_ODBCINST_FILE"; then
58+
if grep -q "^\[$DRIVER_NAME\]" "$ODBCINST_FILE"; then
5959
echo "Driver [$DRIVER_NAME] already exists in odbcinst.ini"
6060
else
6161
echo "Adding [$DRIVER_NAME] to odbcinst.ini..."
6262
echo "
6363
[$DRIVER_NAME]
6464
Description=An ODBC Driver for Apache Arrow Flight SQL
6565
Driver=$ODBC_64BIT
66-
" >>"$USER_ODBCINST_FILE"
66+
" >>"$ODBCINST_FILE"
6767
fi
6868

6969
# Check if [ODBC Drivers] section exists
70-
if grep -q '^\[ODBC Drivers\]' "$USER_ODBCINST_FILE"; then
70+
if grep -q '^\[ODBC Drivers\]' "$ODBCINST_FILE"; then
7171
# Section exists: check if driver entry exists
72-
if ! grep -q "^${DRIVER_NAME}=" "$USER_ODBCINST_FILE"; then
72+
if ! grep -q "^${DRIVER_NAME}=" "$ODBCINST_FILE"; then
7373
# Driver entry does not exist, add under [ODBC Drivers]
7474
sed -i '' "/^\[ODBC Drivers\]/a\\
7575
${DRIVER_NAME}=Installed
76-
" "$USER_ODBCINST_FILE"
76+
" "$ODBCINST_FILE"
7777
fi
7878
else
7979
# Section doesn't exist, append both section and driver entry at end
8080
{
8181
echo ""
8282
echo "[ODBC Drivers]"
8383
echo "${DRIVER_NAME}=Installed"
84-
} >>"$USER_ODBCINST_FILE"
84+
} >>"$ODBCINST_FILE"
8585
fi

‎cpp/src/arrow/flight/sql/odbc/install/unix/install_odbc_ini.sh‎

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -17,17 +17,23 @@
1717
# specific language governing permissions and limitations
1818
# under the License.
1919

20-
SYSTEM_ODBC_FILE="$1"
20+
set -euo pipefail
21+
22+
SYSTEM_ODBC_FILE="${1:-}"
2123

2224
if [[ -z "$SYSTEM_ODBC_FILE" ]]; then
23-
echo "error: path to system ODBC DSN is not specified. Call format: install_odbc_ini abs_path_to_odbc_dsn_ini"
25+
echo "ERROR: path to system ODBC DSN is not specified." >&2
26+
echo "Usage: install_odbc_ini.sh <abs_path_to_odbc_dsn_ini>" >&2
2427
exit 1
2528
fi
2629

2730
DRIVER_NAME="Apache Arrow Flight SQL ODBC Driver"
2831
DSN_NAME="Apache Arrow Flight SQL ODBC DSN"
2932

30-
touch "$SYSTEM_ODBC_FILE"
33+
if ! touch "$SYSTEM_ODBC_FILE"; then
34+
echo "ERROR: Cannot access or create $SYSTEM_ODBC_FILE" >&2
35+
exit 1
36+
fi
3137

3238
if grep -q "^\[$DSN_NAME\]" "$SYSTEM_ODBC_FILE"; then
3339
echo "DSN [$DSN_NAME] already exists in $SYSTEM_ODBC_FILE"
@@ -37,7 +43,7 @@ else
3743
3844
[$DSN_NAME]
3945
Description = An ODBC Driver DSN for Apache Arrow Flight SQL
40-
Driver = Apache Arrow Flight SQL ODBC Driver
46+
Driver = $DRIVER_NAME
4147
Host =
4248
Port =
4349
UID =
@@ -48,8 +54,9 @@ fi
4854
# Check if [ODBC Data Sources] section exists
4955
if grep -q '^\[ODBC Data Sources\]' "$SYSTEM_ODBC_FILE"; then
5056
# Section exists: check if DSN entry exists
51-
if ! grep -q "^${DSN_NAME}=" "$SYSTEM_ODBC_FILE"; then
57+
if ! grep -Eq "^${DSN_NAME}[[:space:]]*=" "$SYSTEM_ODBC_FILE"; then
5258
# Add DSN entry under [ODBC Data Sources] section
59+
tmp_file="$(mktemp "${SYSTEM_ODBC_FILE}.XXXX")"
5360

5461
# Use awk to insert the line immediately after [ODBC Data Sources]
5562
awk -v dsn="$DSN_NAME" -v driver="$DRIVER_NAME" '
@@ -60,7 +67,9 @@ if grep -q '^\[ODBC Data Sources\]' "$SYSTEM_ODBC_FILE"; then
6067
next
6168
}
6269
{ print }
63-
' "$SYSTEM_ODBC_FILE" > "${SYSTEM_ODBC_FILE}.tmp" && mv "${SYSTEM_ODBC_FILE}.tmp" "$SYSTEM_ODBC_FILE"
70+
' "$SYSTEM_ODBC_FILE" > "$tmp_file"
71+
72+
mv "$tmp_file" "$SYSTEM_ODBC_FILE"
6473
fi
6574
else
6675
# Section doesn't exist, append section and DSN entry at end

‎dev/release/rat_exclude_files.txt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ CHANGELOG.md
1414
cpp/CHANGELOG_PARQUET.md
1515
cpp/src/arrow/c/dlpack_abi.h
1616
cpp/src/arrow/flight/sql/odbc/install/mac/README.txt
17-
cpp/src/arrow/flight/sql/odbc/install/mac/Welcome.txt
17+
cpp/src/arrow/flight/sql/odbc/install/mac/welcome.txt
1818
cpp/src/arrow/io/mman.h
1919
cpp/src/arrow/util/random.h
2020
cpp/src/arrow/status.cc

0 commit comments

Comments
 (0)