Skip to content
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.

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

Bump clang-format to v18 and enable auto format #3222

Merged
merged 5 commits into from
Apr 6, 2024
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
The table of contents is too big for display.
Diff view
Diff view
  •  
  •  
  •  
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