Skip to content

Commit eaf55a8

Browse files
justing-bqjusting-bq
committed
Address feedback
1 parent fdd2ef2 commit eaf55a8

8 files changed

Lines changed: 64 additions & 33 deletions

File tree

‎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: 3 additions & 3 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.
7+
4. Enter DSN name and connection string values.
88
For the list of all supported options, check '/Library/ODBC/arrow-odbc/doc/Connection-Options.md'.
9-
5. Click 'Ok' to save the DSN.
9+
5. Click 'OK' to save the DSN.

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

Lines changed: 29 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -17,14 +17,35 @@
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+
#!/usr/bin/env bash
21+
set -euo pipefail
2322

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

28-
# clean temporary script
29-
rm -f "$odbc_install_script"
30-
rm -f "$dsn_install_script"
48+
"$dsn_install_script" "$dsn_file" || {
49+
echo "ERROR: Failed to register DSN to ($dsn_file)" >&2
50+
exit 1
51+
}

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: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -17,17 +17,24 @@
1717
# specific language governing permissions and limitations
1818
# under the License.
1919

20-
SYSTEM_ODBC_FILE="$1"
20+
#!/usr/bin/env bash
21+
set -euo pipefail
22+
23+
SYSTEM_ODBC_FILE="${1:-}"
2124

2225
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"
26+
echo "ERROR: path to system ODBC DSN is not specified." >&2
27+
echo "Usage: install_odbc_ini.sh <abs_path_to_odbc_dsn_ini>" >&2
2428
exit 1
2529
fi
2630

2731
DRIVER_NAME="Apache Arrow Flight SQL ODBC Driver"
2832
DSN_NAME="Apache Arrow Flight SQL ODBC DSN"
2933

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

3239
if grep -q "^\[$DSN_NAME\]" "$SYSTEM_ODBC_FILE"; then
3340
echo "DSN [$DSN_NAME] already exists in $SYSTEM_ODBC_FILE"
@@ -37,7 +44,7 @@ else
3744
3845
[$DSN_NAME]
3946
Description = An ODBC Driver DSN for Apache Arrow Flight SQL
40-
Driver = Apache Arrow Flight SQL ODBC Driver
47+
Driver = $DRIVER_NAME
4148
Host =
4249
Port =
4350
UID =
@@ -48,8 +55,9 @@ fi
4855
# Check if [ODBC Data Sources] section exists
4956
if grep -q '^\[ODBC Data Sources\]' "$SYSTEM_ODBC_FILE"; then
5057
# Section exists: check if DSN entry exists
51-
if ! grep -q "^${DSN_NAME}=" "$SYSTEM_ODBC_FILE"; then
58+
if ! grep -Eq "^${DSN_NAME}[[:space:]]*=" "$SYSTEM_ODBC_FILE"; then
5259
# Add DSN entry under [ODBC Data Sources] section
60+
tmp_file="$(mktemp "${SYSTEM_ODBC_FILE}.XXXX")"
5361

5462
# Use awk to insert the line immediately after [ODBC Data Sources]
5563
awk -v dsn="$DSN_NAME" -v driver="$DRIVER_NAME" '
@@ -60,7 +68,9 @@ if grep -q '^\[ODBC Data Sources\]' "$SYSTEM_ODBC_FILE"; then
6068
next
6169
}
6270
{ print }
63-
' "$SYSTEM_ODBC_FILE" > "${SYSTEM_ODBC_FILE}.tmp" && mv "${SYSTEM_ODBC_FILE}.tmp" "$SYSTEM_ODBC_FILE"
71+
' "$SYSTEM_ODBC_FILE" > "$tmp_file"
72+
73+
mv "$tmp_file" "$SYSTEM_ODBC_FILE"
6474
fi
6575
else
6676
# 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)