Byzanteam / postgrex_wal

1 stars 1 forks source link

feat: parse old tuple data of DELETE, UPDATE #15

Closed json2008 closed 1 year ago

json2008 commented 1 year ago

14

json2008 commented 1 year ago

@vanppo

已按 review 中的意见修改。

请再次 review.

谢谢。

json2008 commented 1 year ago

@fahchen

按 review 中的意见,已经全做了修复。

请再次 review。

谢谢!

json2008 commented 1 year ago
  1. 测试部分你把每个 module 的单元测试做了(从现有的 decoders_test.exs 里面移过去
  2. Message 的继承测试暂时不做,后面集成了数据库后再做

下一个 PR #1, 已经基本写完了。(这个 PR 合并后,立即推上来) 已将 decoder 升级为解码 Protocol 2 格式的消息。同时增加了几个新消息的 decoders.

直接测试 protocol 2 格式的消息。可行?

fahchen commented 1 year ago
  1. 测试部分你把每个 module 的单元测试做了(从现有的 decoders_test.exs 里面移过去

  2. Message 的继承测试暂时不做,后面集成了数据库后再做

下一个 PR #1, 已经基本写完了。(这个 PR 合并后,立即推上来)

已将 decoder 升级为解码 Protocol 2 格式的消息。同时增加了几个新消息的 decoders.

直接测试 protocol 2 格式的消息。可行?

可以,你这个 PR 把测试拆分了

json2008 commented 1 year ago

可以,你这个 PR 把测试拆分了

把测试拆分了?指什么?

目前的测试,能大致对 protocol 1 的解码消息格式做正确测试。

下一个 PR 将对 protocol 2的消息格式做测试。

fahchen commented 1 year ago

可以,你这个 PR 把测试拆分了

把测试拆分了?指什么?

目前的测试,能大致对 protocol 1 的解码消息格式做正确测试。

下一个 PR 将对 protocol 2的消息格式做测试。

https://github.com/Byzanteam/postgrex_wal/pull/15#pullrequestreview-1227084972

json2008 commented 1 year ago

@fahchen

Protocol 2 的代码,合并进来了。(暂缺: 针对 Protocol 2 格式的消息测试。正在补。)

请 review.

json2008 commented 1 year ago

此前的 PR 是针对:不能解码旧数据的问题。(DELETE, UPDATE) 新的 PR 是为了针对 : protocol 2 的新消息格式,以及几个新消息的解码。

现在,合二为一,都在这个 PR 里了。

代码可能有点杂了。

json2008 commented 1 year ago

@fahchen

所有代码,针对官方文档,做了逐一比对。

按 review 给出的意见,做了调整。

请再次 review.

谢谢!

json2008 commented 1 year ago

@fahchen

解码的所有消息,都补上了对应的测试用例。

空时,请再次 review。

谢谢。

json2008 commented 1 year ago

@fahchen

已按 review 中指出的。做出了修改。请再次 review.

json2008 commented 1 year ago

@fahchen

已经修改。请 review。

json2008 commented 1 year ago

Done!

json2008 commented 1 year ago

@fahchen

补上了缺失的测试。

update, delete 的几个 clause 测试覆盖到了。

json2008 commented 1 year ago

当初以为会不同。测试下来,没啥不同。已经去除掉了。以免歧义。

json2008 commented 1 year ago
  @doc """
  LSN (Log Sequence Number) is a pointer to a location in the WAL.

  PostgreSQL uses two representations for the Log Sequence Number (LSN):
  1. Internally, an LSN is a 64-bit integer, representing a byte position in the write-ahead log stream.
     Used internally by PostgreSQL and sent in the XLogData replication messages.

  2. A string of two hexadecimal numbers of up to eight digits each, separated by a slash. e.g. 1/F73E0220.
     This is the form accepted by Postgrex.ReplicationConnection.start_replication/2.
  """
  @spec decode_lsn(lsn :: integer) :: String.t()
  def decode_lsn(lsn) when is_integer(lsn) do
    <<xlog_file_id::32, xlog_offset::32>> = <<lsn::64>>
    Integer.to_string(xlog_file_id, 16) <> "/" <> Integer.to_string(xlog_offset, 16)
  end

@fahchen

从官方文档上看,以及我现在编程中,只用到了这二种形式。所以,解码成: {xlog_file_id, xlog_offset},似乎没有必要。

甚至,上面的 Util.decode_lsn/1,似乎也没必要了,直接使用 Int64 ? (在消息解码阶段)

后面需要使用: XX/XXXX 这种表示形式时,在需要的地方再去 decode/1 ?

fahchen commented 1 year ago

  @doc """

  LSN (Log Sequence Number) is a pointer to a location in the WAL.

  PostgreSQL uses two representations for the Log Sequence Number (LSN):

  1. Internally, an LSN is a 64-bit integer, representing a byte position in the write-ahead log stream.

     Used internally by PostgreSQL and sent in the XLogData replication messages.

  2. A string of two hexadecimal numbers of up to eight digits each, separated by a slash. e.g. 1/F73E0220.

     This is the form accepted by Postgrex.ReplicationConnection.start_replication/2.

  """

  @spec decode_lsn(lsn :: integer) :: String.t()

  def decode_lsn(lsn) when is_integer(lsn) do

    <<xlog_file_id::32, xlog_offset::32>> = <<lsn::64>>

    Integer.to_string(xlog_file_id, 16) <> "/" <> Integer.to_string(xlog_offset, 16)

  end

@fahchen

从官方文档上看,以及我现在编程中,只用到了这二种形式。所以,解码成: {xlog_file_id, xlog_offset},似乎没有必要。

甚至,上面的 Util.decode_lsn/1,似乎也没必要了,直接使用 Int64 ? (在消息解码阶段)

后面需要使用: XX/XXXX 这种表示形式时,在需要的地方再去 decode/1 ?

可以的