Skip to content

Commit

Permalink
Bump clang-format to v18 and enable auto format (#3222)
Browse files Browse the repository at this point in the history
* Bump clang-format to v18 and enable auto format

* Add repository and ref to actions/checkout step

* Update source, test, and extension format checks

* Run clang-format

---------

Co-authored-by: CI Bot <mewim@users.noreply.github.com>
  • Loading branch information
mewim and mewim committed Apr 6, 2024
1 parent 33111c8 commit b3917d9
Show file tree
Hide file tree
Showing 697 changed files with 5,441 additions and 5,377 deletions.
49 changes: 30 additions & 19 deletions .github/workflows/ci-workflow.yml
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ concurrency:
jobs:
gcc-build-test:
name: gcc build & test
needs: [clang-formatting-check, sanity-checks, python-lint-check]
needs: [clang-format, sanity-checks, python-lint-check]
runs-on: kuzu-self-hosted-testing
env:
NUM_THREADS: 32
Expand Down Expand Up @@ -83,7 +83,7 @@ jobs:

gcc-build-test-x86:
name: gcc build & test 32-bit
needs: [clang-formatting-check, sanity-checks]
needs: [clang-format, sanity-checks]
runs-on: ubuntu-latest

steps:
Expand Down Expand Up @@ -157,7 +157,7 @@ jobs:

clang-build-test:
name: clang build and test
needs: [clang-formatting-check, sanity-checks, python-lint-check]
needs: [clang-format, sanity-checks, python-lint-check]
runs-on: kuzu-self-hosted-testing
env:
NUM_THREADS: 32
Expand Down Expand Up @@ -214,7 +214,7 @@ jobs:

msvc-build-test:
name: msvc build & test
needs: [clang-formatting-check, sanity-checks, python-lint-check]
needs: [clang-format, sanity-checks, python-lint-check]
runs-on: self-hosted-windows
env:
# Shorten build path as much as possible
Expand Down Expand Up @@ -285,7 +285,7 @@ jobs:
tidy-and-diagnostics:
name: clang tidy & clangd diagnostics check
needs: [clang-formatting-check, sanity-checks]
needs: [clang-format, sanity-checks]
runs-on: kuzu-self-hosted-testing
env:
NUM_THREADS: 32
Expand Down Expand Up @@ -322,25 +322,37 @@ jobs:
- name: Ensure generated grammar files are up to date
run: cmp src/antlr4/Cypher.g4 scripts/antlr4/Cypher.g4.copy

clang-formatting-check:
name: clang-format check
clang-format:
name: clang format
runs-on: ubuntu-22.04
steps:
- name: Install clang-format
run: |
sudo apt-get update
sudo apt-get install -y clang-format-11
sudo apt-get install -y lsb-release wget software-properties-common gnupg
wget https://apt.llvm.org/llvm.sh
chmod +x llvm.sh
yes | sudo ./llvm.sh 18 all
- uses: actions/checkout@v3
with:
repository: ${{ github.event.pull_request.head.repo.full_name }}
ref: ${{ github.event.pull_request.head.ref }}

- name: Check and fix source format
run: python3 scripts/run-clang-format.py --in-place --clang-format-executable /usr/bin/clang-format-18 -r src/

- name: Check source format
run: python3 scripts/run-clang-format.py --clang-format-executable /usr/bin/clang-format-11 -r src/
- name: Check and fix test format
run: python3 scripts/run-clang-format.py --in-place --clang-format-executable /usr/bin/clang-format-18 -r test/

- name: Check test format
run: python3 scripts/run-clang-format.py --clang-format-executable /usr/bin/clang-format-11 -r test/
- name: Check and fix extension format
run: python3 scripts/run-clang-format.py --in-place --clang-format-executable /usr/bin/clang-format-18 -r extension/

- name: Check extension format
run: python3 scripts/run-clang-format.py --clang-format-executable /usr/bin/clang-format-11 -r extension/
- name: Commit changes
uses: EndBug/add-and-commit@v9
if: github.ref != 'refs/heads/master'
with:
author_name: "CI Bot"
message: "Run clang-format"

python-lint-check:
name: python lint check
Expand Down Expand Up @@ -379,7 +391,7 @@ jobs:

macos-clang-tidy:
name: macos clang tidy & clangd diagnostics check
needs: [clang-formatting-check, sanity-checks]
needs: [clang-format, sanity-checks]
runs-on: self-hosted-mac-x64
env:
NUM_THREADS: 32
Expand All @@ -404,8 +416,7 @@ jobs:

macos-build-test:
name: apple clang build & test
needs:
[clang-formatting-check, sanity-checks, rustfmt-check, python-lint-check]
needs: [clang-format, sanity-checks, rustfmt-check, python-lint-check]
runs-on: self-hosted-mac-x64
env:
NUM_THREADS: 32
Expand Down Expand Up @@ -475,7 +486,7 @@ jobs:
shell-test:
name: shell test
runs-on: ubuntu-latest
needs: [clang-formatting-check, sanity-checks]
needs: [clang-format, sanity-checks]
env:
WERROR: 0
steps:
Expand Down
32 changes: 18 additions & 14 deletions extension/duckdb_scanner/src/duckdb_catalog.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@
namespace kuzu {
namespace duckdb_scanner {

void DuckDBCatalogContent::init(
const std::string& dbPath, const std::string& catalogName, main::ClientContext* context) {
void DuckDBCatalogContent::init(const std::string& dbPath, const std::string& catalogName,
main::ClientContext* context) {
auto [db, con] = getConnection(dbPath);
auto query = common::stringFormat(
"select table_name from information_schema.tables where table_catalog = '{}' and "
Expand All @@ -18,12 +18,14 @@ void DuckDBCatalogContent::init(
std::unique_ptr<duckdb::DataChunk> resultChunk;
try {
resultChunk = result->Fetch();
} catch (std::exception& e) { throw common::BinderException(e.what()); }
} catch (std::exception& e) {
throw common::BinderException(e.what());
}
if (resultChunk->size() == 0) {
return;
}
common::ValueVector tableNamesVector{
*common::LogicalType::STRING(), context->getMemoryManager()};
common::ValueVector tableNamesVector{*common::LogicalType::STRING(),
context->getMemoryManager()};
duckdb_scanner::duckdb_conversion_func_t conversionFunc;
duckdb_scanner::getDuckDBVectorConversionFunc(common::PhysicalTypeID::STRING, conversionFunc);
conversionFunc(resultChunk->data[0], tableNamesVector, resultChunk->size());
Expand All @@ -36,8 +38,8 @@ void DuckDBCatalogContent::init(
static std::string getQuery(const binder::BoundCreateTableInfo& info) {
auto extraInfo = common::ku_dynamic_cast<binder::BoundExtraCreateCatalogEntryInfo*,
BoundExtraCreateDuckDBTableInfo*>(info.extraInfo.get());
return common::stringFormat(
"SELECT * FROM {}.{}.{}", extraInfo->catalogName, extraInfo->schemaName, info.tableName);
return common::stringFormat("SELECT * FROM {}.{}.{}", extraInfo->catalogName,
extraInfo->schemaName, info.tableName);
}

void DuckDBCatalogContent::createForeignTable(duckdb::Connection& con, const std::string& tableName,
Expand All @@ -57,8 +59,8 @@ void DuckDBCatalogContent::createForeignTable(duckdb::Connection& con, const std
}
DuckDBScanBindData bindData(getQuery(*info), std::move(columnTypes), std::move(columnNames),
std::bind(&DuckDBCatalogContent::getConnection, this, dbPath));
auto tableEntry = std::make_unique<catalog::DuckDBTableCatalogEntry>(
info->tableName, tableID, getScanFunction(std::move(bindData)));
auto tableEntry = std::make_unique<catalog::DuckDBTableCatalogEntry>(info->tableName, tableID,
getScanFunction(std::move(bindData)));
for (auto& propertyInfo : extraInfo->propertyInfos) {
tableEntry->addProperty(propertyInfo.name, propertyInfo.type.copy());
}
Expand All @@ -82,7 +84,9 @@ static bool getTableInfo(duckdb::Connection& con, const std::string& tableName,
try {
columnTypes.push_back(DuckDBTypeConverter::convertDuckDBType(
result->GetValue(0, i).GetValue<std::string>()));
} catch (common::BinderException& e) { return false; }
} catch (common::BinderException& e) {
return false;
}
columnNames.push_back(result->GetValue(1, i).GetValue<std::string>());
}
return true;
Expand All @@ -92,8 +96,8 @@ bool DuckDBCatalogContent::bindPropertyInfos(duckdb::Connection& con, const std:
const std::string& catalogName, std::vector<binder::PropertyInfo>& propertyInfos) {
std::vector<common::LogicalType> columnTypes;
std::vector<std::string> columnNames;
if (!getTableInfo(
con, tableName, getDefaultSchemaName(), catalogName, columnTypes, columnNames)) {
if (!getTableInfo(con, tableName, getDefaultSchemaName(), catalogName, columnTypes,
columnNames)) {
return false;
}
for (auto i = 0u; i < columnNames.size(); i++) {
Expand All @@ -111,8 +115,8 @@ std::unique_ptr<binder::BoundCreateTableInfo> DuckDBCatalogContent::bindCreateTa
return nullptr;
}
return std::make_unique<binder::BoundCreateTableInfo>(common::TableType::FOREIGN, tableName,
std::make_unique<duckdb_scanner::BoundExtraCreateDuckDBTableInfo>(
dbPath, catalogName, getDefaultSchemaName(), std::move(propertyInfos)));
std::make_unique<duckdb_scanner::BoundExtraCreateDuckDBTableInfo>(dbPath, catalogName,
getDefaultSchemaName(), std::move(propertyInfos)));
}

std::string DuckDBCatalogContent::getDefaultSchemaName() const {
Expand Down
56 changes: 29 additions & 27 deletions extension/duckdb_scanner/src/duckdb_scan.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ using namespace kuzu::common;
namespace kuzu {
namespace duckdb_scanner {

void getDuckDBVectorConversionFunc(
PhysicalTypeID physicalTypeID, duckdb_conversion_func_t& conversion_func);
void getDuckDBVectorConversionFunc(PhysicalTypeID physicalTypeID,
duckdb_conversion_func_t& conversion_func);

DuckDBScanBindData::DuckDBScanBindData(std::string query,
std::vector<common::LogicalType> columnTypes, std::vector<std::string> columnNames,
Expand All @@ -19,8 +19,8 @@ DuckDBScanBindData::DuckDBScanBindData(std::string query,
initDuckDBConn{std::move(initDuckDBConn)} {
conversionFunctions.resize(this->columnTypes.size());
for (auto i = 0u; i < this->columnTypes.size(); i++) {
getDuckDBVectorConversionFunc(
this->columnTypes[i].getPhysicalType(), conversionFunctions[i]);
getDuckDBVectorConversionFunc(this->columnTypes[i].getPhysicalType(),
conversionFunctions[i]);
}
}

Expand All @@ -34,8 +34,8 @@ DuckDBScanSharedState::DuckDBScanSharedState(std::unique_ptr<duckdb::QueryResult
struct DuckDBScanFunction {
static constexpr char DUCKDB_SCAN_FUNC_NAME[] = "duckdb_scan";

static common::offset_t tableFunc(
function::TableFuncInput& input, function::TableFuncOutput& output);
static common::offset_t tableFunc(function::TableFuncInput& input,
function::TableFuncOutput& output);

static std::unique_ptr<function::TableFuncBindData> bindFunc(DuckDBScanBindData bindData,
main::ClientContext* /*context*/, function::TableFuncBindInput* input);
Expand Down Expand Up @@ -68,8 +68,8 @@ std::unique_ptr<function::TableFuncLocalState> DuckDBScanFunction::initLocalStat
}

template<typename T>
void convertDuckDBVectorToVector(
duckdb::Vector& duckDBVector, ValueVector& result, uint64_t numValuesToCopy) {
void convertDuckDBVectorToVector(duckdb::Vector& duckDBVector, ValueVector& result,
uint64_t numValuesToCopy) {
auto duckDBData = (T*)duckDBVector.GetData();
auto validityMasks = duckdb::FlatVector::Validity(duckDBVector);
memcpy(result.getData(), duckDBData, numValuesToCopy * result.getNumBytesPerValue());
Expand All @@ -79,15 +79,15 @@ void convertDuckDBVectorToVector(
}

template<>
void convertDuckDBVectorToVector<struct_entry_t>(
duckdb::Vector& duckDBVector, ValueVector& result, uint64_t numValuesToCopy);
void convertDuckDBVectorToVector<struct_entry_t>(duckdb::Vector& duckDBVector, ValueVector& result,
uint64_t numValuesToCopy);
template<>
void convertDuckDBVectorToVector<list_entry_t>(
duckdb::Vector& duckDBVector, ValueVector& result, uint64_t numValuesToCopy);
void convertDuckDBVectorToVector<list_entry_t>(duckdb::Vector& duckDBVector, ValueVector& result,
uint64_t numValuesToCopy);

template<>
void convertDuckDBVectorToVector<ku_string_t>(
duckdb::Vector& duckDBVector, ValueVector& result, uint64_t numValuesToCopy) {
void convertDuckDBVectorToVector<ku_string_t>(duckdb::Vector& duckDBVector, ValueVector& result,
uint64_t numValuesToCopy) {
auto strs = reinterpret_cast<duckdb::string_t*>(duckDBVector.GetData());
auto validityMasks = duckdb::FlatVector::Validity(duckDBVector);
for (auto i = 0u; i < numValuesToCopy; i++) {
Expand All @@ -98,8 +98,8 @@ void convertDuckDBVectorToVector<ku_string_t>(
}
}

void getDuckDBVectorConversionFunc(
PhysicalTypeID physicalTypeID, duckdb_conversion_func_t& conversion_func) {
void getDuckDBVectorConversionFunc(PhysicalTypeID physicalTypeID,
duckdb_conversion_func_t& conversion_func) {
switch (physicalTypeID) {
case PhysicalTypeID::BOOL: {
conversion_func = convertDuckDBVectorToVector<bool>;
Expand Down Expand Up @@ -155,10 +155,10 @@ void getDuckDBVectorConversionFunc(
}

template<>
void convertDuckDBVectorToVector<list_entry_t>(
duckdb::Vector& duckDBVector, ValueVector& result, uint64_t numValuesToCopy) {
memcpy(
result.getData(), duckDBVector.GetData(), numValuesToCopy * result.getNumBytesPerValue());
void convertDuckDBVectorToVector<list_entry_t>(duckdb::Vector& duckDBVector, ValueVector& result,
uint64_t numValuesToCopy) {
memcpy(result.getData(), duckDBVector.GetData(),
numValuesToCopy * result.getNumBytesPerValue());
auto numValuesInDataVec = 0;
auto listEntries = reinterpret_cast<duckdb::list_entry_t*>(duckDBVector.GetData());
auto validityMasks = duckdb::FlatVector::Validity(duckDBVector);
Expand All @@ -176,8 +176,8 @@ void convertDuckDBVectorToVector<list_entry_t>(
}

template<>
void convertDuckDBVectorToVector<struct_entry_t>(
duckdb::Vector& duckDBVector, ValueVector& result, uint64_t numValuesToCopy) {
void convertDuckDBVectorToVector<struct_entry_t>(duckdb::Vector& duckDBVector, ValueVector& result,
uint64_t numValuesToCopy) {
auto& duckdbChildrenVectors = duckdb::StructVector::GetEntries(duckDBVector);
for (auto i = 0u; i < duckdbChildrenVectors.size(); i++) {
duckdb_conversion_func_t conversionFunc;
Expand All @@ -193,19 +193,21 @@ static void convertDuckDBResultToVector(duckdb::DataChunk& duckDBResult, DataChu
for (auto i = 0u; i < conversionFuncs.size(); i++) {
result.state->selVector->selectedSize = duckDBResult.size();
assert(duckDBResult.data[i].GetVectorType() == duckdb::VectorType::FLAT_VECTOR);
conversionFuncs[i](
duckDBResult.data[i], *result.getValueVector(i), result.state->selVector->selectedSize);
conversionFuncs[i](duckDBResult.data[i], *result.getValueVector(i),
result.state->selVector->selectedSize);
}
}

common::offset_t DuckDBScanFunction::tableFunc(
function::TableFuncInput& input, function::TableFuncOutput& output) {
common::offset_t DuckDBScanFunction::tableFunc(function::TableFuncInput& input,
function::TableFuncOutput& output) {
auto duckdbScanSharedState = reinterpret_cast<DuckDBScanSharedState*>(input.sharedState);
auto duckdbScanBindData = reinterpret_cast<DuckDBScanBindData*>(input.bindData);
std::unique_ptr<duckdb::DataChunk> result;
try {
result = duckdbScanSharedState->queryResult->Fetch();
} catch (std::exception& e) { return 0; }
} catch (std::exception& e) {
return 0;
}
if (result == nullptr) {
return 0;
}
Expand Down
4 changes: 2 additions & 2 deletions extension/duckdb_scanner/src/duckdb_storage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@
namespace kuzu {
namespace duckdb_scanner {

std::unique_ptr<main::AttachedDatabase> attachDuckDB(
std::string dbName, std::string dbPath, main::ClientContext* clientContext) {
std::unique_ptr<main::AttachedDatabase> attachDuckDB(std::string dbName, std::string dbPath,
main::ClientContext* clientContext) {
if (dbName == "") {
if (dbPath.find('.') != std::string::npos) {
auto fileNamePos = dbPath.find_last_of('/') + 1;
Expand Down
4 changes: 2 additions & 2 deletions extension/duckdb_scanner/src/duckdb_table_catalog_entry.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@
namespace kuzu {
namespace catalog {

DuckDBTableCatalogEntry::DuckDBTableCatalogEntry(
std::string name, common::table_id_t tableID, function::TableFunction scanFunction)
DuckDBTableCatalogEntry::DuckDBTableCatalogEntry(std::string name, common::table_id_t tableID,
function::TableFunction scanFunction)
: TableCatalogEntry{CatalogEntryType::FOREIGN_TABLE_ENTRY, std::move(name), tableID},
scanFunction{std::move(scanFunction)} {}

Expand Down
8 changes: 4 additions & 4 deletions extension/duckdb_scanner/src/duckdb_type_converter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -65,17 +65,17 @@ common::LogicalType DuckDBTypeConverter::convertDuckDBType(std::string typeStr)
return *LogicalType::STRUCT(parseStructTypeInfo(typeStr));
} else if (typeStr.starts_with("UNION")) {
auto unionFields = parseStructTypeInfo(typeStr);
auto unionTagField = StructField(
UnionType::TAG_FIELD_NAME, std::make_unique<LogicalType>(UnionType::TAG_FIELD_TYPE));
auto unionTagField = StructField(UnionType::TAG_FIELD_NAME,
std::make_unique<LogicalType>(UnionType::TAG_FIELD_TYPE));
unionFields.insert(unionFields.begin(), std::move(unionTagField));
return *LogicalType::UNION(std::move(unionFields));
} else if (typeStr.starts_with("MAP")) {
auto leftBracketPos = typeStr.find('(');
auto rightBracketPos = typeStr.find_last_of(')');
auto mapTypeStr = typeStr.substr(leftBracketPos + 1, rightBracketPos - leftBracketPos - 1);
auto keyValueTypes = StringUtils::splitComma(mapTypeStr);
return *LogicalType::MAP(
convertDuckDBType(keyValueTypes[0]), convertDuckDBType(keyValueTypes[1]));
return *LogicalType::MAP(convertDuckDBType(keyValueTypes[0]),
convertDuckDBType(keyValueTypes[1]));
}
throw BinderException{stringFormat("Unsupported duckdb type: {}.", typeStr)};
}
Expand Down
4 changes: 2 additions & 2 deletions extension/duckdb_scanner/src/include/duckdb_catalog.h
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,8 @@ class DuckDBCatalogContent : public catalog::CatalogContent {
public:
DuckDBCatalogContent() : catalog::CatalogContent{nullptr /* vfs */} {}

virtual void init(
const std::string& dbPath, const std::string& catalogName, main::ClientContext* context);
virtual void init(const std::string& dbPath, const std::string& catalogName,
main::ClientContext* context);

protected:
bool bindPropertyInfos(duckdb::Connection& con, const std::string& tableName,
Expand Down
Loading

0 comments on commit b3917d9

Please sign in to comment.