Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
12 changes: 10 additions & 2 deletions .github/workflows/pr-profiler-report.yml
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
name: PR Performance Report

# Privileged reporting only. No PR checkout, builds, or artifact execution here.
# Same-repo PRs may exercise their formatter directly. Forks use trusted base code.
on:
pull_request:
branches: [main]
types: [opened, synchronize, reopened, ready_for_review]
pull_request_target:
branches: [main]
types: [opened, synchronize, reopened, ready_for_review]
Expand All @@ -16,12 +19,17 @@ concurrency:

jobs:
report:
if: >-
(github.event_name == 'pull_request' &&
github.event.pull_request.head.repo.full_name == github.repository) ||
(github.event_name == 'pull_request_target' &&
github.event.pull_request.head.repo.full_name != github.repository)
runs-on: ubuntu-latest
timeout-minutes: 230
steps:
- uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4.4.0
with:
ref: ${{ github.event.pull_request.base.sha }}
ref: ${{ github.event_name == 'pull_request' && github.event.pull_request.head.sha || github.event.pull_request.base.sha }}
Comment thread
jahnvi480 marked this conversation as resolved.
persist-credentials: false
- uses: actions/setup-python@a26af69be951a213d495a4c3e4e4022e16d87065 # v5.6.0
with:
Expand Down
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
does not change the default provider or ship any Rust driver binaries.

### Changed
- DATE, TIME, and TIMESTAMP fetch conversion uses checked CPython constructors
for the standard datetime types, while preserving cached substitute constructors,
their positional arguments and exceptions, and fractional-second truncation.
DATETIMEOFFSET, UUID, and Decimal conversion are unchanged.
- `mssql-python` now depends on `mssql-python-rs==0.1.0` for `mssql_py_core`
instead of embedding files owned by that separately published distribution.
- **GH-769 deprecation policy:** The misplaced `GetInfoConstants` members
Expand Down
12 changes: 6 additions & 6 deletions eng/profiler_benchmarks/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,12 @@ Partial results never produce a verdict.
Two environments publish raw samples: Unix on Ubuntu with SQL Server 2022/2025.
Routine Windows and macOS profiling is intentionally excluded because neutral PRs
showed platform variance above the regression threshold, while both platforms
remain covered by functional CI. The privileged publisher runs
trusted base code, selects the exact PR-head ADO build, and validates bounded
artifacts as data. It publishes as soon as both profiler artifacts exist,
without waiting for unrelated matrix legs. After build completion, missing
artifacts receive a two-minute propagation grace before a partial result is
published. A failed aggregate build can still publish usable profiler artifacts.
remain covered by functional CI. Same-repository PRs run their formatter directly;
fork PRs retain the trusted-base publisher. Both select the exact PR-head ADO build
and validate bounded artifacts as data. Publication begins as soon as both profiler
artifacts exist, without waiting for unrelated matrix legs. After build completion,
missing artifacts receive a two-minute propagation grace before a partial result
is published. A failed aggregate build can still publish usable profiler artifacts.
Exact-head reports may finalize after merge; stale heads are ignored. Missing,
malformed, canceled, incomplete, or invalid data remains unavailable.

Expand Down
151 changes: 95 additions & 56 deletions eng/profiler_benchmarks/report.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
MAX_BYTES = 8 * 1024 * 1024
MAX_COMMENT_CHARS = 60000
MAX_DIAGNOSTIC_ROWS = 20
MAX_FINGERPRINT_TASKS = 4
MARKER = "<!-- mssql-python-profiler-ci -->"
THRESHOLD = 0.20
MIN_DELTA_MS = 1.0
Expand Down Expand Up @@ -376,75 +377,100 @@ def render(reports, head, build_id, issues=()):
]
missing = len(LEGS) - len(completed)

if len(regressions) == 1:
leg, row = regressions[0]
opening = (
f"This PR consistently slows {TASK_NAMES[row['name']].lower()} on "
f"{environment_name(leg)} by {row['change_pct']:.1f}%."
)
elif regressions:
highlighted = [
(leg, row) for leg, (_, rows) in completed.items() for row in rows if row["status"] != "ok"
]
if regressions:
tasks = len({row["name"] for _, row in regressions})
environments = len({leg for leg, _ in regressions})
opening = (
f"This PR has {len(regressions)} consistent slowdown signals across "
f"{tasks} database tasks and {environments} environments."
f"{tasks} database task{'s' if tasks != 1 else ''} consistently slowed down across "
f"{environments} measured environment{'s' if environments != 1 else ''}."
)
verdict = "⚠️ Performance regression detected"
elif noisy:
if len(noisy) == 1:
leg, row = noisy[0]
opening = (
f"{TASK_NAMES[row['name']]} was slower on {environment_name(leg)}, "
"but the repeated comparisons were inconsistent."
)
else:
tasks = len({row["name"] for _, row in noisy})
environments = len({leg for leg, _ in noisy})
opening = (
f"No consistent slowdowns detected. {len(noisy)} inconsistent comparisons "
f"need review across {tasks} database tasks and {environments} environments."
)
elif len(improvements) == 1:
leg, row = improvements[0]
tasks = len({row["name"] for _, row in noisy})
environments = len({leg for leg, _ in noisy})
opening = (
f"This PR consistently makes {TASK_NAMES[row['name']].lower()} faster on "
f"{environment_name(leg)} by {abs(row['change_pct']):.1f}%."
f"{tasks} database task{'s' if tasks != 1 else ''} produced inconsistent slowdown "
f"signals across {environments} measured environment"
f"{'s' if environments != 1 else ''}."
)
verdict = "🔍 Performance needs review"
elif improvements:
tasks = len({row["name"] for _, row in improvements})
environments = len({leg for leg, _ in improvements})
opening = (
f"This PR has {len(improvements)} consistent improvement signals across "
f"{tasks} database tasks and {environments} environments."
f"{tasks} database task{'s' if tasks != 1 else ''} consistently improved across "
f"{environments} measured environment{'s' if environments != 1 else ''}. "
"No consistent slowdowns were detected."
)
verdict = "✅ Performance improved"
elif not completed:
opening = (
"Performance could not be assessed because no environment produced a complete result."
)
verdict = "⛔ Performance unavailable"
elif not missing:
opening = f"No consistent slowdowns detected across all {len(LEGS)} environments."
verdict = "✅ No regression detected"
else:
completed_label = "environment" if len(completed) == 1 else "environments"
missing_label = "environment" if missing == 1 else "environments"
opening = (
f"No consistent slowdowns in the {len(completed)} completed {completed_label}. "
f"No result is available for {missing} {missing_label}."
)
verdict = "✅ No regression detected"

lines = [MARKER, "## PR Performance Report", "", f"**{opening}**", ""]
highlighted = regressions or noisy or improvements
if highlighted:
if not regressions and noisy:
lines += ["Inconsistent slowdowns to review:", ""]
improvement_tasks = len({row["name"] for _, row in improvements})
regression_tasks = len({row["name"] for _, row in regressions})
lines = [
MARKER,
"## PR Performance Report",
"",
f"### {verdict}",
"",
f"**{opening}**",
"",
f"<kbd>{improvement_tasks} IMPROVEMENT"
f"{'S' if improvement_tasks != 1 else ''}</kbd> "
f"<kbd>{regression_tasks} SLOWDOWN"
f"{'S' if regression_tasks != 1 else ''}</kbd> "
f"<kbd>{len(completed)}/{len(LEGS)} ENVIRONMENTS</kbd>",
"",
]
if noisy:
noisy_tasks = len({row["name"] for _, row in noisy})
lines += [
"| Environment | Affected task | Before | After | Change |",
"|---|---|---:|---:|---:|",
f"<kbd>{noisy_tasks} INCONSISTENT SLOWDOWN" f"{'S' if noisy_tasks != 1 else ''}</kbd>",
"",
]
for leg, row in highlighted:
lines.append(
f"| {environment_name(leg)} | {TASK_NAMES[row['name']]} | "
f"{row['base_ms']:.3f} ms | {row['candidate_ms']:.3f} ms | "
f"{row['change_pct']:+.1f}% |"
)
affected_tasks = [name for name in CASES if any(row["name"] == name for _, row in highlighted)]
if highlighted and len(affected_tasks) <= MAX_FINGERPRINT_TASKS:
affected_legs = [leg for leg in LEGS if any(item_leg == leg for item_leg, _ in highlighted)]
by_signal = {(leg, row["name"]): row for leg, row in highlighted}
lines += [
"### Signal fingerprint",
"",
"| Database task | "
+ " | ".join(environment_name(leg) for leg in affected_legs)
+ " |",
"|---|" + "|".join("---:" for _ in affected_legs) + "|",
]
for name in affected_tasks:
cells = []
for leg in affected_legs:
row = by_signal.get((leg, name))
if row is None:
cells.append("No signal")
elif row["status"] == "improvement":
cells.append(f"**{abs(row['change_pct']):.1f}% faster**")
elif row["status"] == "regression":
cells.append(f"**{abs(row['change_pct']):.1f}% slower**")
else:
cells.append(f"**{abs(row['change_pct']):.1f}% inconsistent**")
lines.append(f"| {escape(TASK_NAMES[name])} | " + " | ".join(cells) + " |")
lines.append("")
if regressions:
lines.append(
Expand All @@ -461,24 +487,37 @@ def render(reports, head, build_id, issues=()):
lines += [
f"**Coverage:** {len(completed)} of {len(LEGS)} environments completed. "
"Advisory result; does not block merging.",
"",
"| Environment | Status |",
"|---|---|",
]
for leg in LEGS:
report = by_leg.get(leg)
status = (
"Completed"
if leg in completed
else f"No result available ({escape(issue_reason(leg, issues))})"
)
lines.append(f"| {environment_name(leg)} | {status} |")
unavailable_legs = [
f"{environment_name(leg)} ({escape(issue_reason(leg, issues))})"
for leg in LEGS
if leg not in completed
]
if unavailable_legs:
lines += ["", "Unavailable: " + "; ".join(unavailable_legs) + "."]

if highlighted:
lines += [
"",
"<details>",
"<summary><b>Measured timings</b></summary>",
"",
"| Environment | Database task | Before | After | Change |",
"|---|---|---:|---:|---:|",
]
for leg, row in highlighted:
lines.append(
f"| {environment_name(leg)} | {TASK_NAMES[row['name']]} | "
f"{row['base_ms']:.3f} ms | {row['candidate_ms']:.3f} ms | "
f"**{row['change_pct']:+.1f}%** |"
)
lines += ["", "</details>"]

diagnostics_start = len(lines)
lines += [
"",
"<details>",
"<summary>Affected phases and call counts</summary>",
"<summary><b>Performance diagnostics</b></summary>",
"",
"Phase times are inclusive diagnostics and must not be added together. "
"They identify where measured time changed, not why it changed.",
Expand Down Expand Up @@ -516,7 +555,7 @@ def render(reports, head, build_id, issues=()):
lines += [
"",
"<details>",
"<summary>All database tasks and timings</summary>",
"<summary><b>All database tasks and timings</b></summary>",
]

for leg, (report, rows) in completed.items():
Expand All @@ -542,7 +581,7 @@ def render(reports, head, build_id, issues=()):
"</details>",
"",
"<details>",
"<summary>Build, commits and measurement details</summary>",
"<summary><b>Build and measurement details</b></summary>",
"",
]
lines += [
Expand Down Expand Up @@ -591,7 +630,7 @@ def render(reports, head, build_id, issues=()):
lines[diagnostics_start:diagnostics_end] = [
"",
"<details>",
"<summary>Affected phases and call counts</summary>",
"<summary><b>Performance diagnostics</b></summary>",
"",
f"{total_diagnostics} diagnostic rows are available in the raw ADO artifacts.",
"",
Expand Down
38 changes: 15 additions & 23 deletions mssql_python/pybind/ddbc_bindings.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include "param_detect.hpp"
#include "py_ref.hpp"
#include "py_type_cache.hpp"
#include "fetch_temporal.hpp"
#include "row_factory.hpp"
#include "utf_utils.h"
#include "fetch_text.hpp"
Expand Down Expand Up @@ -3766,8 +3767,8 @@ SQLRETURN SQLGetData_wrap(SqlHandlePtr StatementHandle, SQLUSMALLINT colCount, p
ret = SQLGetData_ptr(hStmt, i, SQL_C_TYPE_DATE, &dateValue, sizeof(dateValue),
&indicator);
if (SQL_SUCCEEDED(ret) && indicator != SQL_NULL_DATA) {
row.append(PyTypeCache::get_date_class_obj()(dateValue.year, dateValue.month,
dateValue.day));
row.append(
FetchTemporal::date(dateValue.year, dateValue.month, dateValue.day));
} else {
row.append(py::none());
}
Expand All @@ -3779,7 +3780,7 @@ SQLRETURN SQLGetData_wrap(SqlHandlePtr StatementHandle, SQLUSMALLINT colCount, p
SQLLEN indicator = 0;
ret = SQLGetData_ptr(hStmt, i, SQL_C_SS_TIME2, &t2, sizeof(t2), &indicator);
if (SQL_SUCCEEDED(ret) && indicator != SQL_NULL_DATA) {
row.append(PyTypeCache::get_time_class_obj()(
row.append(FetchTemporal::time(
t2.hour, t2.minute, t2.second, t2.fraction / 1000)); // ns to µs
} else {
if (!SQL_SUCCEEDED(ret)) {
Expand All @@ -3803,7 +3804,7 @@ SQLRETURN SQLGetData_wrap(SqlHandlePtr StatementHandle, SQLUSMALLINT colCount, p
break;
}
if (SQL_SUCCEEDED(ret)) {
row.append(PyTypeCache::get_datetime_class_obj()(
row.append(FetchTemporal::datetime(
timestampValue.year, timestampValue.month, timestampValue.day,
timestampValue.hour, timestampValue.minute, timestampValue.second,
timestampValue.fraction / 1000 // Convert back ns to µs
Expand Down Expand Up @@ -4451,32 +4452,23 @@ SQLRETURN FetchBatchData(SQLHSTMT hStmt, ColumnBuffers& buffers, py::list& colum
case SQL_TYPE_TIMESTAMP:
case SQL_DATETIME: {
const SQL_TIMESTAMP_STRUCT& ts = buffers.timestampBuffers[col - 1][i];
PyObject* datetimeObj = PyTypeCache::get_datetime_class_obj()(
ts.year, ts.month, ts.day, ts.hour, ts.minute,
ts.second, ts.fraction / 1000)
.release()
.ptr();
PyList_SET_ITEM(row, col - 1, datetimeObj);
py::object datetimeObj = FetchTemporal::datetime(
ts.year, ts.month, ts.day, ts.hour, ts.minute, ts.second,
ts.fraction / 1000);
PyList_SET_ITEM(row, col - 1, datetimeObj.release().ptr());
break;
}
case SQL_TYPE_DATE: {
PyObject* dateObj =
PyTypeCache::get_date_class_obj()(buffers.dateBuffers[col - 1][i].year,
buffers.dateBuffers[col - 1][i].month,
buffers.dateBuffers[col - 1][i].day)
.release()
.ptr();
PyList_SET_ITEM(row, col - 1, dateObj);
const SQL_DATE_STRUCT& value = buffers.dateBuffers[col - 1][i];
py::object dateObj = FetchTemporal::date(value.year, value.month, value.day);
PyList_SET_ITEM(row, col - 1, dateObj.release().ptr());
break;
}
case SQL_SS_TIME2: {
const SQL_SS_TIME2_STRUCT& t2 = buffers.timeBuffers[col - 1][i];
PyObject* timeObj =
PyTypeCache::get_time_class_obj()(t2.hour, t2.minute, t2.second,
t2.fraction / 1000) // ns to µs
.release()
.ptr();
PyList_SET_ITEM(row, col - 1, timeObj);
py::object timeObj =
FetchTemporal::time(t2.hour, t2.minute, t2.second, t2.fraction / 1000);
PyList_SET_ITEM(row, col - 1, timeObj.release().ptr());
break;
}
case SQL_SS_TIMESTAMPOFFSET: {
Expand Down
Loading
Loading