graphql-python / graphql-python/graphene-sqlalchemy

Automatic conversion from Int to ID is problematic

未關閉
#102 15 則留言 14 個 reaction 已指派 0 人 在 GitHub 檢視
主要語言
Python
星號
985
分支
223
PR 合併指標
30 天內沒有已合併 PR

描述

Graphene-SQLAlchemy automatically converts columns of type SmallInteger or Integer to `ID!` fields if they are primary keys, but does not convert such columns to `ID` fields if they are foreign keys.

Take for example this schema:
```python
class Department(Base):
__tablename__ = 'department'
id = Column(Integer, primary_key=True)
name = Column(String)

class User(Base):
__tablename__ = 'users'
id = Column(Integer, primary_key=True)
name = Column(String)
department_id = Column(Integer, ForeignKey('department.id'))
department = relationship(Department)

class DepartmentType(SQLAlchemyObjectType):
class Meta:
model = Department

class UserType(SQLAlchemyObjectType):
class Meta:
model = User

class Query(ObjectType):

departments = List(DepartmentType)
users = List(UserType)

def resolve_departments(self, info):
return DepartmentType.get_query(info)

def resolve_users(self, info):
return UserType.get_query(info)
```
You can run the following query:
```graphql
query {
users {
id
name
departmentId
department {
id
}
}
}
```
As a result, you get something like:
```json
{
"data": {
"users": [
{
"id": "1",
"firstName": "Fred",
"departmentId": 1,
"department": {
"id": "1"
}
},
{
"id": "2",
"firstName": "Barnie",
"departmentId": 2,
"department": {
"id": "2"
}
}
]
}
}
```
As you see, `department.id` is a string (because IDs are returned as strings), while `departmentId` is a number. This turned out to be a huge problem and source of error in practice. Working with this inconsistent, fault-prone interface has bitten me many times. When storing ids in objects on the frontend, or using ids as filters, I never know whether I should use numbers or strings. Currently I have conversions from number to string and vice versa everywhere in my frontend code, and if I don't do it correctly, things stop working in hard to debug ways because you often don't recognize such type mismatches. On the server side, do I take ids used as filter parameters as IDs or Ints? If I do the former, I must then convert them to integer when using them as filter arguments for SQLAlchemy. So, really, this is no fun to work with and doesn't work in practice, because you always have this mental burden of thinking about whether your ids should be represented as strings or numbers and whether you need to convert them when passing them around.

I suggest the conversions should be consistent. Either convert all keys, including foreign keys, to IDs, or do not make a special case conversion for primary keys. Actually I'd prefer the latter, since then I never need to think about the type and since storing numbers on the frontend uses less memory.

Now of course I know that there is the relay specification which assumes there is an `id` field with a type of `ID`. So when using the relay interface, things are different. In this case, I suggest converting to IDs everywhere (including foreign keys) - but here we need conversion of the values to global ids anyway, they are not just the row ids converted to strings.

貢獻指南

開啟貢獻指南

研究方向

該 payload 沒有指出來源檔案或測試;首先使用 Department 和 User 範例,追蹤 SQLAlchemyObjectType 為主鍵和外鍵整數欄位產生 schema 的過程。完成的標準是,專案已針對這些欄位決定一致的 ID 與 Int 取捨策略,並為所示的 GraphQL 查詢提供回歸涵蓋率。

由索引模型根據 Issue 內容生成。

評估

技術堆疊
graphql, python, sqlalchemy
領域
api, backend, database
Issue 類型
功能
難度
5/5
預估耗時
一週以上
活躍度
停滯
描述清晰度
基本清楚
新手友好度
30/100

把新 issue 寄到你的電子郵件信箱

精選適合新手參與的 GitHub issue 摘要。